* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-15 0:09 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-15 0:09 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> > Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi usage")
> > broke support for setting cmdline on Nokia N900 via
> > CONFIG_CMDLINE.
> >
> > It is because arm code booted in DT mode parse cmdline only via
> > function early_init_dt_scan_chosen() and that function does not
> > fill variable boot_command_line when DTB does not contain /chosen
> > entry. It is called from function early_init_dt_scan_nodes() in
> > setup_machine_fdt().
> >
> > This patch fixes it by explicitly filling boot_command_line in
> > function setup_machine_fdt() after calling
> > early_init_dt_scan_nodes() in case boot_command_line still remains
> > empty.
>
> This looks like a hack.
>
> First, the matter of the ATAGs compatibility. The decompressor
> relies on there being a pre-existing /chosen node to insert the
> command line and other parameters into. If we've dropped it (by
> dropping skeleton.dtsi) then we've just regressed more than just
> N900 - the decompressor won't be able to merge the ATAGs into the
> concatenated FDT.
Hm... I did not think about it. But right this can be broken too...
> Second, CONFIG_CMDLINE has never been in place on DT systems - it's
> something atags_parse.c has been dealing with which isn't involved on
> DT. For DT, we expect the command line to be passed in.
If cmdline is not in DT, but /chosen exists, then function
early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
> Now, given that, I find your commit message rather fishy. You seem
> to be claiming that CONFIG_CMDLINE used to work, but the fact is,
> we've never had it working on device tree systems.
Looks like that CONFIG_CMDLINE really did not worked (correctly) on DT
booted systems... We have already discussed about it on #armlinux
> Instead, what I think was going on is that the bug you're seeing is
> that the removal of skeleton.dtsi results in the /chosen node being
> absent, which in turn prevents the decompressor picking the various
> parameters out of the ATAGs and dropping them into the DT blob.
In my case, bootloader did not provided any cmdline in ATAG, so
decompressor did not parsed any cmdline...
> That, to me, sounds like a regression, and the fix is not to hack
> support for an unrelated feature, but to fix the original problem -
> and I think in this case, it's reasonable to ask for the offending
> commit to be reverted.
I will let decision to others, who created and accepted that patch
008a2ebcd677. What I need is working CONFIG_CMDLINE as I had it before
as bootloader for Nokia N900 does not pass any cmdline -- and without it
I cannot boot userspace.
But still, I would like to see working CONFIG_CMDLINE support for DT
systems. Other systems (e.g. arm ATAG based or x86) have it.
What is reason that CONFIG_CMDLINE is not supported for DT?
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161215/19624c67/attachment-0001.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-15 0:09 ` Pali Rohár
@ 2016-12-15 0:18 ` Robin Murphy
-1 siblings, 0 replies; 44+ messages in thread
From: Robin Murphy @ 2016-12-15 0:18 UTC (permalink / raw)
To: Pali Rohár
Cc: Russell King - ARM Linux, Arnd Bergmann, Linus Walleij,
Ben Dooks, Tony Lindgren, Ivaylo Dimitrov, Sebastian Reichel,
Aaro Koskinen, Pavel Machek, linux-arm-kernel, linux-kernel
On Thu, 15 Dec 2016 01:09:20 +0100
Pali Rohár <pali.rohar@gmail.com> wrote:
> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
> > On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
> > > Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> > > usage") broke support for setting cmdline on Nokia N900 via
> > > CONFIG_CMDLINE.
> > >
> > > It is because arm code booted in DT mode parse cmdline only via
> > > function early_init_dt_scan_chosen() and that function does not
> > > fill variable boot_command_line when DTB does not contain /chosen
> > > entry. It is called from function early_init_dt_scan_nodes() in
> > > setup_machine_fdt().
> > >
> > > This patch fixes it by explicitly filling boot_command_line in
> > > function setup_machine_fdt() after calling
> > > early_init_dt_scan_nodes() in case boot_command_line still remains
> > > empty.
> >
> > This looks like a hack.
> >
> > First, the matter of the ATAGs compatibility. The decompressor
> > relies on there being a pre-existing /chosen node to insert the
> > command line and other parameters into. If we've dropped it (by
> > dropping skeleton.dtsi) then we've just regressed more than just
> > N900 - the decompressor won't be able to merge the ATAGs into the
> > concatenated FDT.
>
> Hm... I did not think about it. But right this can be broken too...
>
> > Second, CONFIG_CMDLINE has never been in place on DT systems - it's
> > something atags_parse.c has been dealing with which isn't involved
> > on DT. For DT, we expect the command line to be passed in.
>
> If cmdline is not in DT, but /chosen exists, then function
> early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
>
> > Now, given that, I find your commit message rather fishy. You seem
> > to be claiming that CONFIG_CMDLINE used to work, but the fact is,
> > we've never had it working on device tree systems.
>
> Looks like that CONFIG_CMDLINE really did not worked (correctly) on
> DT booted systems... We have already discussed about it on #armlinux
>
> > Instead, what I think was going on is that the bug you're seeing is
> > that the removal of skeleton.dtsi results in the /chosen node being
> > absent, which in turn prevents the decompressor picking the various
> > parameters out of the ATAGs and dropping them into the DT blob.
>
> In my case, bootloader did not provided any cmdline in ATAG, so
> decompressor did not parsed any cmdline...
>
> > That, to me, sounds like a regression, and the fix is not to hack
> > support for an unrelated feature, but to fix the original problem -
> > and I think in this case, it's reasonable to ask for the offending
> > commit to be reverted.
>
> I will let decision to others, who created and accepted that patch
> 008a2ebcd677. What I need is working CONFIG_CMDLINE as I had it
> before as bootloader for Nokia N900 does not pass any cmdline -- and
> without it I cannot boot userspace.
Isn't that what CONFIG_CMDLINE_FORCE is for?
Or you could, y'know, just add a chosen node to the N900 DT to put it
back in the same state as before...
Robin.
> But still, I would like to see working CONFIG_CMDLINE support for DT
> systems. Other systems (e.g. arm ATAG based or x86) have it.
>
> What is reason that CONFIG_CMDLINE is not supported for DT?
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-15 0:18 ` Robin Murphy
0 siblings, 0 replies; 44+ messages in thread
From: Robin Murphy @ 2016-12-15 0:18 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 15 Dec 2016 01:09:20 +0100
Pali Roh?r <pali.rohar@gmail.com> wrote:
> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
> > On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> > > Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> > > usage") broke support for setting cmdline on Nokia N900 via
> > > CONFIG_CMDLINE.
> > >
> > > It is because arm code booted in DT mode parse cmdline only via
> > > function early_init_dt_scan_chosen() and that function does not
> > > fill variable boot_command_line when DTB does not contain /chosen
> > > entry. It is called from function early_init_dt_scan_nodes() in
> > > setup_machine_fdt().
> > >
> > > This patch fixes it by explicitly filling boot_command_line in
> > > function setup_machine_fdt() after calling
> > > early_init_dt_scan_nodes() in case boot_command_line still remains
> > > empty.
> >
> > This looks like a hack.
> >
> > First, the matter of the ATAGs compatibility. The decompressor
> > relies on there being a pre-existing /chosen node to insert the
> > command line and other parameters into. If we've dropped it (by
> > dropping skeleton.dtsi) then we've just regressed more than just
> > N900 - the decompressor won't be able to merge the ATAGs into the
> > concatenated FDT.
>
> Hm... I did not think about it. But right this can be broken too...
>
> > Second, CONFIG_CMDLINE has never been in place on DT systems - it's
> > something atags_parse.c has been dealing with which isn't involved
> > on DT. For DT, we expect the command line to be passed in.
>
> If cmdline is not in DT, but /chosen exists, then function
> early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
>
> > Now, given that, I find your commit message rather fishy. You seem
> > to be claiming that CONFIG_CMDLINE used to work, but the fact is,
> > we've never had it working on device tree systems.
>
> Looks like that CONFIG_CMDLINE really did not worked (correctly) on
> DT booted systems... We have already discussed about it on #armlinux
>
> > Instead, what I think was going on is that the bug you're seeing is
> > that the removal of skeleton.dtsi results in the /chosen node being
> > absent, which in turn prevents the decompressor picking the various
> > parameters out of the ATAGs and dropping them into the DT blob.
>
> In my case, bootloader did not provided any cmdline in ATAG, so
> decompressor did not parsed any cmdline...
>
> > That, to me, sounds like a regression, and the fix is not to hack
> > support for an unrelated feature, but to fix the original problem -
> > and I think in this case, it's reasonable to ask for the offending
> > commit to be reverted.
>
> I will let decision to others, who created and accepted that patch
> 008a2ebcd677. What I need is working CONFIG_CMDLINE as I had it
> before as bootloader for Nokia N900 does not pass any cmdline -- and
> without it I cannot boot userspace.
Isn't that what CONFIG_CMDLINE_FORCE is for?
Or you could, y'know, just add a chosen node to the N900 DT to put it
back in the same state as before...
Robin.
> But still, I would like to see working CONFIG_CMDLINE support for DT
> systems. Other systems (e.g. arm ATAG based or x86) have it.
>
> What is reason that CONFIG_CMDLINE is not supported for DT?
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-15 0:09 ` Pali Rohár
@ 2016-12-15 10:09 ` Russell King - ARM Linux
-1 siblings, 0 replies; 44+ messages in thread
From: Russell King - ARM Linux @ 2016-12-15 10:09 UTC (permalink / raw)
To: Pali Rohár
Cc: Arnd Bergmann, Robin Murphy, Linus Walleij, Ben Dooks,
Tony Lindgren, Ivaylo Dimitrov, Sebastian Reichel, Aaro Koskinen,
Pavel Machek, linux-arm-kernel, linux-kernel
On Thu, Dec 15, 2016 at 01:09:20AM +0100, Pali Rohár wrote:
> If cmdline is not in DT, but /chosen exists, then function
> early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
Ah, yes. Looks to me then as if the bug exists there, and not in arch
code then. early_init_dt_scan_chosen() completely ignores the CMDLINE
options if the chosen node is not found.
> What is reason that CONFIG_CMDLINE is not supported for DT?
Sorry, that's my mistake - as you've pointed out above, it is supported
but via generic code. I was only looking at arch code when I made the
statement.
This patch (untested) should solve it:
drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index c89d5d231a0e..fb89157332c6 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1073,26 +1073,6 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
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
-#if defined(CONFIG_CMDLINE_EXTEND)
- strlcat(data, " ", COMMAND_LINE_SIZE);
- strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#else
- /* No arguments from boot loader, use kernel's cmdl*/
- if (!((char *)data)[0])
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
-#endif /* CONFIG_CMDLINE */
-
- pr_debug("Command line is: %s\n", (char*)data);
-
/* break now */
return 1;
}
@@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
/* Retrieve various information from the /chosen node */
of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
+ /*
+ * 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
+#if defined(CONFIG_CMDLINE_EXTEND)
+ strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+ strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#elif defined(CONFIG_CMDLINE_FORCE)
+ strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#else
+ /* No arguments from boot loader, use kernel's cmdline */
+ if (!boot_command_line[0])
+ strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#endif
+#endif /* CONFIG_CMDLINE */
+
+ pr_debug("Command line is: %s\n", boot_command_line);
+
/* Initialize {size,address}-cells info */
of_scan_flat_dt(early_init_dt_scan_root, NULL);
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-15 10:09 ` Russell King - ARM Linux
0 siblings, 0 replies; 44+ messages in thread
From: Russell King - ARM Linux @ 2016-12-15 10:09 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 15, 2016 at 01:09:20AM +0100, Pali Roh?r wrote:
> If cmdline is not in DT, but /chosen exists, then function
> early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
Ah, yes. Looks to me then as if the bug exists there, and not in arch
code then. early_init_dt_scan_chosen() completely ignores the CMDLINE
options if the chosen node is not found.
> What is reason that CONFIG_CMDLINE is not supported for DT?
Sorry, that's my mistake - as you've pointed out above, it is supported
but via generic code. I was only looking at arch code when I made the
statement.
This patch (untested) should solve it:
drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index c89d5d231a0e..fb89157332c6 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1073,26 +1073,6 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
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
-#if defined(CONFIG_CMDLINE_EXTEND)
- strlcat(data, " ", COMMAND_LINE_SIZE);
- strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#else
- /* No arguments from boot loader, use kernel's cmdl*/
- if (!((char *)data)[0])
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
-#endif /* CONFIG_CMDLINE */
-
- pr_debug("Command line is: %s\n", (char*)data);
-
/* break now */
return 1;
}
@@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
/* Retrieve various information from the /chosen node */
of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
+ /*
+ * 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
+#if defined(CONFIG_CMDLINE_EXTEND)
+ strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+ strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#elif defined(CONFIG_CMDLINE_FORCE)
+ strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#else
+ /* No arguments from boot loader, use kernel's cmdline */
+ if (!boot_command_line[0])
+ strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#endif
+#endif /* CONFIG_CMDLINE */
+
+ pr_debug("Command line is: %s\n", boot_command_line);
+
/* Initialize {size,address}-cells info */
of_scan_flat_dt(early_init_dt_scan_root, NULL);
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-15 10:09 ` Russell King - ARM Linux
@ 2016-12-16 11:42 ` Pali Rohár
-1 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 11:42 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Arnd Bergmann, Robin Murphy, Linus Walleij, Ben Dooks,
Tony Lindgren, Ivaylo Dimitrov, Sebastian Reichel, Aaro Koskinen,
Pavel Machek, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 3485 bytes --]
On Thursday 15 December 2016 11:09:32 Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2016 at 01:09:20AM +0100, Pali Rohár wrote:
> > If cmdline is not in DT, but /chosen exists, then function
> > early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
>
> Ah, yes. Looks to me then as if the bug exists there, and not in
> arch code then. early_init_dt_scan_chosen() completely ignores the
> CMDLINE options if the chosen node is not found.
Yes... but question is: shoud function named early_init_dt_scan_chosen()
use CONFIG_CMDLINE if there is DT entry? From name dt_scan_chosen I
would expect that it scan /chosen and set some fields from /chosen...
At least name of that function is confusing.
> > What is reason that CONFIG_CMDLINE is not supported for DT?
>
> Sorry, that's my mistake - as you've pointed out above, it is
> supported but via generic code. I was only looking at arch code
> when I made the statement.
>
> This patch (untested) should solve it:
>
> drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c89d5d231a0e..fb89157332c6 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1073,26 +1073,6 @@ int __init early_init_dt_scan_chosen(unsigned
> long node, const char *uname, 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
> -#if defined(CONFIG_CMDLINE_EXTEND)
> - strlcat(data, " ", COMMAND_LINE_SIZE);
> - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#elif defined(CONFIG_CMDLINE_FORCE)
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#else
> - /* No arguments from boot loader, use kernel's cmdl*/
> - if (!((char *)data)[0])
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#endif
> -#endif /* CONFIG_CMDLINE */
> -
> - pr_debug("Command line is: %s\n", (char*)data);
> -
> /* break now */
> return 1;
> }
> @@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
> /* Retrieve various information from the /chosen node */
> of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
>
> + /*
> + * 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
> +#if defined(CONFIG_CMDLINE_EXTEND)
> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> + strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#elif defined(CONFIG_CMDLINE_FORCE)
> + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#else
> + /* No arguments from boot loader, use kernel's cmdline */
> + if (!boot_command_line[0])
> + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#endif
> +#endif /* CONFIG_CMDLINE */
> +
> + pr_debug("Command line is: %s\n", boot_command_line);
> +
> /* Initialize {size,address}-cells info */
> of_scan_flat_dt(early_init_dt_scan_root, NULL);
Patch is working fine and fixes my problem. You can add my Tested-by.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 11:42 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 11:42 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 15 December 2016 11:09:32 Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2016 at 01:09:20AM +0100, Pali Roh?r wrote:
> > If cmdline is not in DT, but /chosen exists, then function
> > early_init_dt_scan_chosen() use cmdline from CONFIG_CMDLINE.
>
> Ah, yes. Looks to me then as if the bug exists there, and not in
> arch code then. early_init_dt_scan_chosen() completely ignores the
> CMDLINE options if the chosen node is not found.
Yes... but question is: shoud function named early_init_dt_scan_chosen()
use CONFIG_CMDLINE if there is DT entry? From name dt_scan_chosen I
would expect that it scan /chosen and set some fields from /chosen...
At least name of that function is confusing.
> > What is reason that CONFIG_CMDLINE is not supported for DT?
>
> Sorry, that's my mistake - as you've pointed out above, it is
> supported but via generic code. I was only looking at arch code
> when I made the statement.
>
> This patch (untested) should solve it:
>
> drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c89d5d231a0e..fb89157332c6 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1073,26 +1073,6 @@ int __init early_init_dt_scan_chosen(unsigned
> long node, const char *uname, 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
> -#if defined(CONFIG_CMDLINE_EXTEND)
> - strlcat(data, " ", COMMAND_LINE_SIZE);
> - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#elif defined(CONFIG_CMDLINE_FORCE)
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#else
> - /* No arguments from boot loader, use kernel's cmdl*/
> - if (!((char *)data)[0])
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#endif
> -#endif /* CONFIG_CMDLINE */
> -
> - pr_debug("Command line is: %s\n", (char*)data);
> -
> /* break now */
> return 1;
> }
> @@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
> /* Retrieve various information from the /chosen node */
> of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
>
> + /*
> + * 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
> +#if defined(CONFIG_CMDLINE_EXTEND)
> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> + strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#elif defined(CONFIG_CMDLINE_FORCE)
> + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#else
> + /* No arguments from boot loader, use kernel's cmdline */
> + if (!boot_command_line[0])
> + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#endif
> +#endif /* CONFIG_CMDLINE */
> +
> + pr_debug("Command line is: %s\n", boot_command_line);
> +
> /* Initialize {size,address}-cells info */
> of_scan_flat_dt(early_init_dt_scan_root, NULL);
Patch is working fine and fixes my problem. You can add my Tested-by.
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/bfd7ea52/attachment.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 11:42 ` Pali Rohár
@ 2016-12-25 22:08 ` Pali Rohár
-1 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-25 22:08 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Arnd Bergmann, Robin Murphy, Linus Walleij, Ben Dooks,
Tony Lindgren, Ivaylo Dimitrov, Sebastian Reichel, Aaro Koskinen,
Pavel Machek, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 3264 bytes --]
On Friday 16 December 2016 12:42:34 Pali Rohár wrote:
> On Thursday 15 December 2016 11:09:32 Russell King - ARM Linux wrote:
> > > What is reason that CONFIG_CMDLINE is not supported for DT?
> >
> > Sorry, that's my mistake - as you've pointed out above, it is
> > supported but via generic code. I was only looking at arch code
> > when I made the statement.
> >
> > This patch (untested) should solve it:
> > drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
> > 1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index c89d5d231a0e..fb89157332c6 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -1073,26 +1073,6 @@ int __init
> > early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > 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
> > -#if defined(CONFIG_CMDLINE_EXTEND)
> > - strlcat(data, " ", COMMAND_LINE_SIZE);
> > - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#elif defined(CONFIG_CMDLINE_FORCE)
> > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#else
> > - /* No arguments from boot loader, use kernel's cmdl*/
> > - if (!((char *)data)[0])
> > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#endif
> > -#endif /* CONFIG_CMDLINE */
> > -
> > - pr_debug("Command line is: %s\n", (char*)data);
> > -
> >
> > /* break now */
> > return 1;
> >
> > }
> >
> > @@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
> >
> > /* Retrieve various information from the /chosen node */
> > of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
> >
> > + /*
> > + * 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
> > +#if defined(CONFIG_CMDLINE_EXTEND)
> > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > + strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#elif defined(CONFIG_CMDLINE_FORCE)
> > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#else
> > + /* No arguments from boot loader, use kernel's cmdline */
> > + if (!boot_command_line[0])
> > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#endif
> > +#endif /* CONFIG_CMDLINE */
> > +
> > + pr_debug("Command line is: %s\n", boot_command_line);
> > +
> >
> > /* Initialize {size,address}-cells info */
> > of_scan_flat_dt(early_init_dt_scan_root, NULL);
>
> Patch is working fine and fixes my problem. You can add my Tested-by.
Russell, are you going to use your patch? Or original my? Or any other?
If you need more testing or other data, let me know.
I would like to see this problem (non working CONFIG_CMDLINE) fixed...
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-25 22:08 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-25 22:08 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 16 December 2016 12:42:34 Pali Roh?r wrote:
> On Thursday 15 December 2016 11:09:32 Russell King - ARM Linux wrote:
> > > What is reason that CONFIG_CMDLINE is not supported for DT?
> >
> > Sorry, that's my mistake - as you've pointed out above, it is
> > supported but via generic code. I was only looking at arch code
> > when I made the statement.
> >
> > This patch (untested) should solve it:
> > drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
> > 1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index c89d5d231a0e..fb89157332c6 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -1073,26 +1073,6 @@ int __init
> > early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > 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
> > -#if defined(CONFIG_CMDLINE_EXTEND)
> > - strlcat(data, " ", COMMAND_LINE_SIZE);
> > - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#elif defined(CONFIG_CMDLINE_FORCE)
> > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#else
> > - /* No arguments from boot loader, use kernel's cmdl*/
> > - if (!((char *)data)[0])
> > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#endif
> > -#endif /* CONFIG_CMDLINE */
> > -
> > - pr_debug("Command line is: %s\n", (char*)data);
> > -
> >
> > /* break now */
> > return 1;
> >
> > }
> >
> > @@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
> >
> > /* Retrieve various information from the /chosen node */
> > of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
> >
> > + /*
> > + * 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
> > +#if defined(CONFIG_CMDLINE_EXTEND)
> > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > + strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#elif defined(CONFIG_CMDLINE_FORCE)
> > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#else
> > + /* No arguments from boot loader, use kernel's cmdline */
> > + if (!boot_command_line[0])
> > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > +#endif
> > +#endif /* CONFIG_CMDLINE */
> > +
> > + pr_debug("Command line is: %s\n", boot_command_line);
> > +
> >
> > /* Initialize {size,address}-cells info */
> > of_scan_flat_dt(early_init_dt_scan_root, NULL);
>
> Patch is working fine and fixes my problem. You can add my Tested-by.
Russell, are you going to use your patch? Or original my? Or any other?
If you need more testing or other data, let me know.
I would like to see this problem (non working CONFIG_CMDLINE) fixed...
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161225/1962bb36/attachment.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-25 22:08 ` Pali Rohár
@ 2017-01-02 13:54 ` Russell King - ARM Linux
-1 siblings, 0 replies; 44+ messages in thread
From: Russell King - ARM Linux @ 2017-01-02 13:54 UTC (permalink / raw)
To: Pali Rohár
Cc: Arnd Bergmann, Robin Murphy, Linus Walleij, Ben Dooks,
Tony Lindgren, Ivaylo Dimitrov, Sebastian Reichel, Aaro Koskinen,
Pavel Machek, linux-arm-kernel, linux-kernel
On Sun, Dec 25, 2016 at 11:08:15PM +0100, Pali Rohár wrote:
> On Friday 16 December 2016 12:42:34 Pali Rohár wrote:
> > On Thursday 15 December 2016 11:09:32 Russell King - ARM Linux wrote:
> > > > What is reason that CONFIG_CMDLINE is not supported for DT?
> > >
> > > Sorry, that's my mistake - as you've pointed out above, it is
> > > supported but via generic code. I was only looking at arch code
> > > when I made the statement.
> > >
> > > This patch (untested) should solve it:
> > > drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
> > > 1 file changed, 20 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index c89d5d231a0e..fb89157332c6 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -1073,26 +1073,6 @@ int __init
> > > early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > > 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
> > > -#if defined(CONFIG_CMDLINE_EXTEND)
> > > - strlcat(data, " ", COMMAND_LINE_SIZE);
> > > - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > -#elif defined(CONFIG_CMDLINE_FORCE)
> > > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > -#else
> > > - /* No arguments from boot loader, use kernel's cmdl*/
> > > - if (!((char *)data)[0])
> > > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > -#endif
> > > -#endif /* CONFIG_CMDLINE */
> > > -
> > > - pr_debug("Command line is: %s\n", (char*)data);
> > > -
> > >
> > > /* break now */
> > > return 1;
> > >
> > > }
> > >
> > > @@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
> > >
> > > /* Retrieve various information from the /chosen node */
> > > of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
> > >
> > > + /*
> > > + * 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
> > > +#if defined(CONFIG_CMDLINE_EXTEND)
> > > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > > + strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > +#elif defined(CONFIG_CMDLINE_FORCE)
> > > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > +#else
> > > + /* No arguments from boot loader, use kernel's cmdline */
> > > + if (!boot_command_line[0])
> > > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > +#endif
> > > +#endif /* CONFIG_CMDLINE */
> > > +
> > > + pr_debug("Command line is: %s\n", boot_command_line);
> > > +
> > >
> > > /* Initialize {size,address}-cells info */
> > > of_scan_flat_dt(early_init_dt_scan_root, NULL);
> >
> > Patch is working fine and fixes my problem. You can add my Tested-by.
>
> Russell, are you going to use your patch? Or original my? Or any other?
> If you need more testing or other data, let me know.
>
> I would like to see this problem (non working CONFIG_CMDLINE) fixed...
If the DT people would like to comment on it and give me an ack or take
the patch... I guess that's not going to happen as they're not on the
Cc... I'll resend later today.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2017-01-02 13:54 ` Russell King - ARM Linux
0 siblings, 0 replies; 44+ messages in thread
From: Russell King - ARM Linux @ 2017-01-02 13:54 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Dec 25, 2016 at 11:08:15PM +0100, Pali Roh?r wrote:
> On Friday 16 December 2016 12:42:34 Pali Roh?r wrote:
> > On Thursday 15 December 2016 11:09:32 Russell King - ARM Linux wrote:
> > > > What is reason that CONFIG_CMDLINE is not supported for DT?
> > >
> > > Sorry, that's my mistake - as you've pointed out above, it is
> > > supported but via generic code. I was only looking at arch code
> > > when I made the statement.
> > >
> > > This patch (untested) should solve it:
> > > drivers/of/fdt.c | 40 ++++++++++++++++++++--------------------
> > > 1 file changed, 20 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index c89d5d231a0e..fb89157332c6 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -1073,26 +1073,6 @@ int __init
> > > early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > > 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
> > > -#if defined(CONFIG_CMDLINE_EXTEND)
> > > - strlcat(data, " ", COMMAND_LINE_SIZE);
> > > - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > -#elif defined(CONFIG_CMDLINE_FORCE)
> > > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > -#else
> > > - /* No arguments from boot loader, use kernel's cmdl*/
> > > - if (!((char *)data)[0])
> > > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > -#endif
> > > -#endif /* CONFIG_CMDLINE */
> > > -
> > > - pr_debug("Command line is: %s\n", (char*)data);
> > > -
> > >
> > > /* break now */
> > > return 1;
> > >
> > > }
> > >
> > > @@ -1205,6 +1185,26 @@ void __init early_init_dt_scan_nodes(void)
> > >
> > > /* Retrieve various information from the /chosen node */
> > > of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
> > >
> > > + /*
> > > + * 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
> > > +#if defined(CONFIG_CMDLINE_EXTEND)
> > > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > > + strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > +#elif defined(CONFIG_CMDLINE_FORCE)
> > > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > +#else
> > > + /* No arguments from boot loader, use kernel's cmdline */
> > > + if (!boot_command_line[0])
> > > + strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > +#endif
> > > +#endif /* CONFIG_CMDLINE */
> > > +
> > > + pr_debug("Command line is: %s\n", boot_command_line);
> > > +
> > >
> > > /* Initialize {size,address}-cells info */
> > > of_scan_flat_dt(early_init_dt_scan_root, NULL);
> >
> > Patch is working fine and fixes my problem. You can add my Tested-by.
>
> Russell, are you going to use your patch? Or original my? Or any other?
> If you need more testing or other data, let me know.
>
> I would like to see this problem (non working CONFIG_CMDLINE) fixed...
If the DT people would like to comment on it and give me an ack or take
the patch... I guess that's not going to happen as they're not on the
Cc... I'll resend later today.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-15 0:09 ` Pali Rohár
@ 2016-12-16 11:46 ` Pali Rohár
-1 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 11:46 UTC (permalink / raw)
To: Tony Lindgren, Javier Martinez Canillas
Cc: Russell King - ARM Linux, Arnd Bergmann, Robin Murphy,
Linus Walleij, Ben Dooks, Ivaylo Dimitrov, Sebastian Reichel,
Aaro Koskinen, Pavel Machek, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 1595 bytes --]
On Thursday 15 December 2016 01:09:20 Pali Rohár wrote:
> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
> > On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
> > > Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> > > usage") broke support for setting cmdline on Nokia N900 via
> > > CONFIG_CMDLINE.
> > >
> > > It is because arm code booted in DT mode parse cmdline only via
> > > function early_init_dt_scan_chosen() and that function does not
> > > fill variable boot_command_line when DTB does not contain /chosen
> > > entry. It is called from function early_init_dt_scan_nodes() in
> > > setup_machine_fdt().
> > >
> > > This patch fixes it by explicitly filling boot_command_line in
> > > function setup_machine_fdt() after calling
> > > early_init_dt_scan_nodes() in case boot_command_line still
> > > remains empty.
> >
> > This looks like a hack.
> >
> > First, the matter of the ATAGs compatibility. The decompressor
> > relies on there being a pre-existing /chosen node to insert the
> > command line and other parameters into. If we've dropped it (by
> > dropping skeleton.dtsi) then we've just regressed more than just
> > N900 - the decompressor won't be able to merge the ATAGs into the
> > concatenated FDT.
>
> Hm... I did not think about it. But right this can be broken too...
Tony, Javier: are you aware of above ↑↑↑ problem?
It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
skeleton.dtsi usage") should be really reverted.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 11:46 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 11:46 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
> > On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> > > Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> > > usage") broke support for setting cmdline on Nokia N900 via
> > > CONFIG_CMDLINE.
> > >
> > > It is because arm code booted in DT mode parse cmdline only via
> > > function early_init_dt_scan_chosen() and that function does not
> > > fill variable boot_command_line when DTB does not contain /chosen
> > > entry. It is called from function early_init_dt_scan_nodes() in
> > > setup_machine_fdt().
> > >
> > > This patch fixes it by explicitly filling boot_command_line in
> > > function setup_machine_fdt() after calling
> > > early_init_dt_scan_nodes() in case boot_command_line still
> > > remains empty.
> >
> > This looks like a hack.
> >
> > First, the matter of the ATAGs compatibility. The decompressor
> > relies on there being a pre-existing /chosen node to insert the
> > command line and other parameters into. If we've dropped it (by
> > dropping skeleton.dtsi) then we've just regressed more than just
> > N900 - the decompressor won't be able to merge the ATAGs into the
> > concatenated FDT.
>
> Hm... I did not think about it. But right this can be broken too...
Tony, Javier: are you aware of above ??? problem?
It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
skeleton.dtsi usage") should be really reverted.
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/cf408722/attachment.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 11:46 ` Pali Rohár
@ 2016-12-16 12:13 ` Javier Martinez Canillas
-1 siblings, 0 replies; 44+ messages in thread
From: Javier Martinez Canillas @ 2016-12-16 12:13 UTC (permalink / raw)
To: Pali Rohár, Tony Lindgren
Cc: Russell King - ARM Linux, Arnd Bergmann, Robin Murphy,
Linus Walleij, Ben Dooks, Ivaylo Dimitrov, Sebastian Reichel,
Aaro Koskinen, Pavel Machek, linux-arm-kernel, linux-kernel
Hello Pali,
On 12/16/2016 08:46 AM, Pali Rohár wrote:
> On Thursday 15 December 2016 01:09:20 Pali Rohár wrote:
>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>> CONFIG_CMDLINE.
>>>>
>>>> It is because arm code booted in DT mode parse cmdline only via
>>>> function early_init_dt_scan_chosen() and that function does not
>>>> fill variable boot_command_line when DTB does not contain /chosen
>>>> entry. It is called from function early_init_dt_scan_nodes() in
>>>> setup_machine_fdt().
>>>>
>>>> This patch fixes it by explicitly filling boot_command_line in
>>>> function setup_machine_fdt() after calling
>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>> remains empty.
>>>
>>> This looks like a hack.
>>>
>>> First, the matter of the ATAGs compatibility. The decompressor
>>> relies on there being a pre-existing /chosen node to insert the
>>> command line and other parameters into. If we've dropped it (by
>>> dropping skeleton.dtsi) then we've just regressed more than just
>>> N900 - the decompressor won't be able to merge the ATAGs into the
>>> concatenated FDT.
>>
>> Hm... I did not think about it. But right this can be broken too...
>
> Tony, Javier: are you aware of above ↑↑↑ problem?
>
> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> skeleton.dtsi usage") should be really reverted.
>
I don't think reverting the mentioned commit is the correct fix for your
problem. We are trying to get rid of skeleton.dtsi for many reasons, see
commit commit ("3ebee5a2e141 arm64: dts: kill skeleton.dtsi").
Also, the chosen node is mentioned to be optional in the ePAPR document
and u-boot creates a chosen node if isn't found [0] so this issue is only
present in boards that don't use u-boot like the N900/N950/N9 phones.
So if NOLO doesn't do the same than u-boot and the kernel expects a chosen
node, I suggest to add an empty chosen node in the omap3-n900.dts and
omap3-n950-n9.dtsi device tree source files.
[0]: http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 12:13 ` Javier Martinez Canillas
0 siblings, 0 replies; 44+ messages in thread
From: Javier Martinez Canillas @ 2016-12-16 12:13 UTC (permalink / raw)
To: linux-arm-kernel
Hello Pali,
On 12/16/2016 08:46 AM, Pali Roh?r wrote:
> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux wrote:
>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>> CONFIG_CMDLINE.
>>>>
>>>> It is because arm code booted in DT mode parse cmdline only via
>>>> function early_init_dt_scan_chosen() and that function does not
>>>> fill variable boot_command_line when DTB does not contain /chosen
>>>> entry. It is called from function early_init_dt_scan_nodes() in
>>>> setup_machine_fdt().
>>>>
>>>> This patch fixes it by explicitly filling boot_command_line in
>>>> function setup_machine_fdt() after calling
>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>> remains empty.
>>>
>>> This looks like a hack.
>>>
>>> First, the matter of the ATAGs compatibility. The decompressor
>>> relies on there being a pre-existing /chosen node to insert the
>>> command line and other parameters into. If we've dropped it (by
>>> dropping skeleton.dtsi) then we've just regressed more than just
>>> N900 - the decompressor won't be able to merge the ATAGs into the
>>> concatenated FDT.
>>
>> Hm... I did not think about it. But right this can be broken too...
>
> Tony, Javier: are you aware of above ??? problem?
>
> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> skeleton.dtsi usage") should be really reverted.
>
I don't think reverting the mentioned commit is the correct fix for your
problem. We are trying to get rid of skeleton.dtsi for many reasons, see
commit commit ("3ebee5a2e141 arm64: dts: kill skeleton.dtsi").
Also, the chosen node is mentioned to be optional in the ePAPR document
and u-boot creates a chosen node if isn't found [0] so this issue is only
present in boards that don't use u-boot like the N900/N950/N9 phones.
So if NOLO doesn't do the same than u-boot and the kernel expects a chosen
node, I suggest to add an empty chosen node in the omap3-n900.dts and
omap3-n950-n9.dtsi device tree source files.
[0]: http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 12:13 ` Javier Martinez Canillas
@ 2016-12-16 12:32 ` Pali Rohár
-1 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 12:32 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Tony Lindgren, Russell King - ARM Linux, Arnd Bergmann,
Robin Murphy, Linus Walleij, Ben Dooks, Ivaylo Dimitrov,
Sebastian Reichel, Aaro Koskinen, Pavel Machek, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 4144 bytes --]
Hi!
On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
> Hello Pali,
>
> On 12/16/2016 08:46 AM, Pali Rohár wrote:
> > On Thursday 15 December 2016 01:09:20 Pali Rohár wrote:
> >> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
> >> wrote:
> >>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
> >>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> >>>> usage") broke support for setting cmdline on Nokia N900 via
> >>>> CONFIG_CMDLINE.
> >>>>
> >>>> It is because arm code booted in DT mode parse cmdline only via
> >>>> function early_init_dt_scan_chosen() and that function does not
> >>>> fill variable boot_command_line when DTB does not contain
> >>>> /chosen entry. It is called from function
> >>>> early_init_dt_scan_nodes() in setup_machine_fdt().
> >>>>
> >>>> This patch fixes it by explicitly filling boot_command_line in
> >>>> function setup_machine_fdt() after calling
> >>>> early_init_dt_scan_nodes() in case boot_command_line still
> >>>> remains empty.
> >>>
> >>> This looks like a hack.
> >>>
> >>> First, the matter of the ATAGs compatibility. The decompressor
> >>> relies on there being a pre-existing /chosen node to insert the
> >>> command line and other parameters into. If we've dropped it (by
> >>> dropping skeleton.dtsi) then we've just regressed more than just
> >>> N900 - the decompressor won't be able to merge the ATAGs into the
> >>> concatenated FDT.
> >>
> >> Hm... I did not think about it. But right this can be broken
> >> too...
> >
> > Tony, Javier: are you aware of above ↑↑↑ problem?
> >
> > It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> > skeleton.dtsi usage") should be really reverted.
>
> I don't think reverting the mentioned commit is the correct fix for
> your problem. We are trying to get rid of skeleton.dtsi for many
> reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
> skeleton.dtsi").
$ git show 3ebee5a2e141
* The default empty /chosen and /aliases are somewhat useless...
That is not truth, they are not useless as Russell King wrote --
removing them break ATAG support.
(But that commit is for arm64 which probably is not using ATAGs... But I
do not know. At least it is not truth for 32bit arm.)
> Also, the chosen node is mentioned to be optional in the ePAPR
> document and u-boot creates a chosen node if isn't found [0] so this
> issue is only present in boards that don't use u-boot like the
> N900/N950/N9 phones.
Linux arm decompressor does not propagate ATAGs when /chosen is missing.
Sorry, but if for Linux /chosen is required (and without it is broken!)
then some ePARP document does not apply there. Either Linux code needs
to be fixed (so /chosen will be really only optional) or /chosen stay in
Linux required. There is no other option.
And I hope that U-boot is not the only one bootloader which Linux kernel
supports. I thought that I can use *any* bootloader to boot Linux kernel
not just U-Boot which is doing some magic...
With this step you are basically going to break booting Linux kernel
with all others bootloaders... And personally I really dislike this
idea.
> So if NOLO doesn't do the same than u-boot and the kernel expects a
> chosen node, I suggest to add an empty chosen node in the
> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
That would fix a problem for N900, N950 and N9. But not for all other
ARM devices which bootloader pass some ATAGs.
IIRC rule of kernel is not to break compatibility and that commit
008a2ebcd677 really did it.
Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying
that it cause problems which need to be properly fixed. And if fixing
them is harder and will take more time, then correct option is to revert
008a2ebcd677 due to breaking support for more devices.
> [0]:
> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f
> 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>
> Best regards,
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 12:32 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 12:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi!
On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
> Hello Pali,
>
> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
> > On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
> >> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
> >> wrote:
> >>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> >>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> >>>> usage") broke support for setting cmdline on Nokia N900 via
> >>>> CONFIG_CMDLINE.
> >>>>
> >>>> It is because arm code booted in DT mode parse cmdline only via
> >>>> function early_init_dt_scan_chosen() and that function does not
> >>>> fill variable boot_command_line when DTB does not contain
> >>>> /chosen entry. It is called from function
> >>>> early_init_dt_scan_nodes() in setup_machine_fdt().
> >>>>
> >>>> This patch fixes it by explicitly filling boot_command_line in
> >>>> function setup_machine_fdt() after calling
> >>>> early_init_dt_scan_nodes() in case boot_command_line still
> >>>> remains empty.
> >>>
> >>> This looks like a hack.
> >>>
> >>> First, the matter of the ATAGs compatibility. The decompressor
> >>> relies on there being a pre-existing /chosen node to insert the
> >>> command line and other parameters into. If we've dropped it (by
> >>> dropping skeleton.dtsi) then we've just regressed more than just
> >>> N900 - the decompressor won't be able to merge the ATAGs into the
> >>> concatenated FDT.
> >>
> >> Hm... I did not think about it. But right this can be broken
> >> too...
> >
> > Tony, Javier: are you aware of above ??? problem?
> >
> > It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> > skeleton.dtsi usage") should be really reverted.
>
> I don't think reverting the mentioned commit is the correct fix for
> your problem. We are trying to get rid of skeleton.dtsi for many
> reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
> skeleton.dtsi").
$ git show 3ebee5a2e141
* The default empty /chosen and /aliases are somewhat useless...
That is not truth, they are not useless as Russell King wrote --
removing them break ATAG support.
(But that commit is for arm64 which probably is not using ATAGs... But I
do not know. At least it is not truth for 32bit arm.)
> Also, the chosen node is mentioned to be optional in the ePAPR
> document and u-boot creates a chosen node if isn't found [0] so this
> issue is only present in boards that don't use u-boot like the
> N900/N950/N9 phones.
Linux arm decompressor does not propagate ATAGs when /chosen is missing.
Sorry, but if for Linux /chosen is required (and without it is broken!)
then some ePARP document does not apply there. Either Linux code needs
to be fixed (so /chosen will be really only optional) or /chosen stay in
Linux required. There is no other option.
And I hope that U-boot is not the only one bootloader which Linux kernel
supports. I thought that I can use *any* bootloader to boot Linux kernel
not just U-Boot which is doing some magic...
With this step you are basically going to break booting Linux kernel
with all others bootloaders... And personally I really dislike this
idea.
> So if NOLO doesn't do the same than u-boot and the kernel expects a
> chosen node, I suggest to add an empty chosen node in the
> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
That would fix a problem for N900, N950 and N9. But not for all other
ARM devices which bootloader pass some ATAGs.
IIRC rule of kernel is not to break compatibility and that commit
008a2ebcd677 really did it.
Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying
that it cause problems which need to be properly fixed. And if fixing
them is harder and will take more time, then correct option is to revert
008a2ebcd677 due to breaking support for more devices.
> [0]:
> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f
> 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>
> Best regards,
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/e81662a3/attachment-0001.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 12:32 ` Pali Rohár
@ 2016-12-16 12:38 ` Javier Martinez Canillas
-1 siblings, 0 replies; 44+ messages in thread
From: Javier Martinez Canillas @ 2016-12-16 12:38 UTC (permalink / raw)
To: Pali Rohár
Cc: Tony Lindgren, Russell King - ARM Linux, Arnd Bergmann,
Robin Murphy, Linus Walleij, Ben Dooks, Ivaylo Dimitrov,
Sebastian Reichel, Aaro Koskinen, Pavel Machek, linux-arm-kernel,
linux-kernel
Hello Pali,
On 12/16/2016 09:32 AM, Pali Rohár wrote:
> Hi!
>
> On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
>> Hello Pali,
>>
>> On 12/16/2016 08:46 AM, Pali Rohár wrote:
>>> On Thursday 15 December 2016 01:09:20 Pali Rohár wrote:
>>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
>>>> wrote:
>>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
>>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>>>> CONFIG_CMDLINE.
>>>>>>
>>>>>> It is because arm code booted in DT mode parse cmdline only via
>>>>>> function early_init_dt_scan_chosen() and that function does not
>>>>>> fill variable boot_command_line when DTB does not contain
>>>>>> /chosen entry. It is called from function
>>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
>>>>>>
>>>>>> This patch fixes it by explicitly filling boot_command_line in
>>>>>> function setup_machine_fdt() after calling
>>>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>>>> remains empty.
>>>>>
>>>>> This looks like a hack.
>>>>>
>>>>> First, the matter of the ATAGs compatibility. The decompressor
>>>>> relies on there being a pre-existing /chosen node to insert the
>>>>> command line and other parameters into. If we've dropped it (by
>>>>> dropping skeleton.dtsi) then we've just regressed more than just
>>>>> N900 - the decompressor won't be able to merge the ATAGs into the
>>>>> concatenated FDT.
>>>>
>>>> Hm... I did not think about it. But right this can be broken
>>>> too...
>>>
>>> Tony, Javier: are you aware of above ↑↑↑ problem?
>>>
>>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
>>> skeleton.dtsi usage") should be really reverted.
>>
>> I don't think reverting the mentioned commit is the correct fix for
>> your problem. We are trying to get rid of skeleton.dtsi for many
>> reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
>> skeleton.dtsi").
>
> $ git show 3ebee5a2e141
>
> * The default empty /chosen and /aliases are somewhat useless...
>
> That is not truth, they are not useless as Russell King wrote --
> removing them break ATAG support.
>
> (But that commit is for arm64 which probably is not using ATAGs... But I
> do not know. At least it is not truth for 32bit arm.)
>
>> Also, the chosen node is mentioned to be optional in the ePAPR
>> document and u-boot creates a chosen node if isn't found [0] so this
>> issue is only present in boards that don't use u-boot like the
>> N900/N950/N9 phones.
>
> Linux arm decompressor does not propagate ATAGs when /chosen is missing.
> Sorry, but if for Linux /chosen is required (and without it is broken!)
> then some ePARP document does not apply there. Either Linux code needs
> to be fixed (so /chosen will be really only optional) or /chosen stay in
> Linux required. There is no other option.
>
> And I hope that U-boot is not the only one bootloader which Linux kernel
> supports. I thought that I can use *any* bootloader to boot Linux kernel
> not just U-Boot which is doing some magic...
>
> With this step you are basically going to break booting Linux kernel
> with all others bootloaders... And personally I really dislike this
> idea.
>
>> So if NOLO doesn't do the same than u-boot and the kernel expects a
>> chosen node, I suggest to add an empty chosen node in the
>> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
>
> That would fix a problem for N900, N950 and N9. But not for all other
> ARM devices which bootloader pass some ATAGs.
>
> IIRC rule of kernel is not to break compatibility and that commit
> 008a2ebcd677 really did it.
>
> Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying
> that it cause problems which need to be properly fixed. And if fixing
> them is harder and will take more time, then correct option is to revert
> 008a2ebcd677 due to breaking support for more devices.
>
If you think that others boards may have the same issue, then you could
add an empty chosen node to omap3.dtsi. As I said I think that in practice
this will only be needed for the machines using NOLO but you are right
that in theory you could boot them using other bootloaders and having an
empty node doesn't cause any harm anyway.
>> [0]:
>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f
>> 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>>
>> Best regards,
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 12:38 ` Javier Martinez Canillas
0 siblings, 0 replies; 44+ messages in thread
From: Javier Martinez Canillas @ 2016-12-16 12:38 UTC (permalink / raw)
To: linux-arm-kernel
Hello Pali,
On 12/16/2016 09:32 AM, Pali Roh?r wrote:
> Hi!
>
> On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
>> Hello Pali,
>>
>> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
>>> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
>>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
>>>> wrote:
>>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
>>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>>>> CONFIG_CMDLINE.
>>>>>>
>>>>>> It is because arm code booted in DT mode parse cmdline only via
>>>>>> function early_init_dt_scan_chosen() and that function does not
>>>>>> fill variable boot_command_line when DTB does not contain
>>>>>> /chosen entry. It is called from function
>>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
>>>>>>
>>>>>> This patch fixes it by explicitly filling boot_command_line in
>>>>>> function setup_machine_fdt() after calling
>>>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>>>> remains empty.
>>>>>
>>>>> This looks like a hack.
>>>>>
>>>>> First, the matter of the ATAGs compatibility. The decompressor
>>>>> relies on there being a pre-existing /chosen node to insert the
>>>>> command line and other parameters into. If we've dropped it (by
>>>>> dropping skeleton.dtsi) then we've just regressed more than just
>>>>> N900 - the decompressor won't be able to merge the ATAGs into the
>>>>> concatenated FDT.
>>>>
>>>> Hm... I did not think about it. But right this can be broken
>>>> too...
>>>
>>> Tony, Javier: are you aware of above ??? problem?
>>>
>>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
>>> skeleton.dtsi usage") should be really reverted.
>>
>> I don't think reverting the mentioned commit is the correct fix for
>> your problem. We are trying to get rid of skeleton.dtsi for many
>> reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
>> skeleton.dtsi").
>
> $ git show 3ebee5a2e141
>
> * The default empty /chosen and /aliases are somewhat useless...
>
> That is not truth, they are not useless as Russell King wrote --
> removing them break ATAG support.
>
> (But that commit is for arm64 which probably is not using ATAGs... But I
> do not know. At least it is not truth for 32bit arm.)
>
>> Also, the chosen node is mentioned to be optional in the ePAPR
>> document and u-boot creates a chosen node if isn't found [0] so this
>> issue is only present in boards that don't use u-boot like the
>> N900/N950/N9 phones.
>
> Linux arm decompressor does not propagate ATAGs when /chosen is missing.
> Sorry, but if for Linux /chosen is required (and without it is broken!)
> then some ePARP document does not apply there. Either Linux code needs
> to be fixed (so /chosen will be really only optional) or /chosen stay in
> Linux required. There is no other option.
>
> And I hope that U-boot is not the only one bootloader which Linux kernel
> supports. I thought that I can use *any* bootloader to boot Linux kernel
> not just U-Boot which is doing some magic...
>
> With this step you are basically going to break booting Linux kernel
> with all others bootloaders... And personally I really dislike this
> idea.
>
>> So if NOLO doesn't do the same than u-boot and the kernel expects a
>> chosen node, I suggest to add an empty chosen node in the
>> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
>
> That would fix a problem for N900, N950 and N9. But not for all other
> ARM devices which bootloader pass some ATAGs.
>
> IIRC rule of kernel is not to break compatibility and that commit
> 008a2ebcd677 really did it.
>
> Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying
> that it cause problems which need to be properly fixed. And if fixing
> them is harder and will take more time, then correct option is to revert
> 008a2ebcd677 due to breaking support for more devices.
>
If you think that others boards may have the same issue, then you could
add an empty chosen node to omap3.dtsi. As I said I think that in practice
this will only be needed for the machines using NOLO but you are right
that in theory you could boot them using other bootloaders and having an
empty node doesn't cause any harm anyway.
>> [0]:
>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c9f
>> 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>>
>> Best regards,
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 12:38 ` Javier Martinez Canillas
@ 2016-12-16 12:48 ` Pali Rohár
-1 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 12:48 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Tony Lindgren, Russell King - ARM Linux, Arnd Bergmann,
Robin Murphy, Linus Walleij, Ben Dooks, Ivaylo Dimitrov,
Sebastian Reichel, Aaro Koskinen, Pavel Machek, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 5042 bytes --]
On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote:
> Hello Pali,
>
> On 12/16/2016 09:32 AM, Pali Rohár wrote:
> > Hi!
> >
> > On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
> >> Hello Pali,
> >>
> >> On 12/16/2016 08:46 AM, Pali Rohár wrote:
> >>> On Thursday 15 December 2016 01:09:20 Pali Rohár wrote:
> >>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
> >>>>
> >>>> wrote:
> >>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
> >>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> >>>>>> usage") broke support for setting cmdline on Nokia N900 via
> >>>>>> CONFIG_CMDLINE.
> >>>>>>
> >>>>>> It is because arm code booted in DT mode parse cmdline only
> >>>>>> via function early_init_dt_scan_chosen() and that function
> >>>>>> does not fill variable boot_command_line when DTB does not
> >>>>>> contain /chosen entry. It is called from function
> >>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
> >>>>>>
> >>>>>> This patch fixes it by explicitly filling boot_command_line in
> >>>>>> function setup_machine_fdt() after calling
> >>>>>> early_init_dt_scan_nodes() in case boot_command_line still
> >>>>>> remains empty.
> >>>>>
> >>>>> This looks like a hack.
> >>>>>
> >>>>> First, the matter of the ATAGs compatibility. The decompressor
> >>>>> relies on there being a pre-existing /chosen node to insert the
> >>>>> command line and other parameters into. If we've dropped it
> >>>>> (by dropping skeleton.dtsi) then we've just regressed more
> >>>>> than just N900 - the decompressor won't be able to merge the
> >>>>> ATAGs into the concatenated FDT.
> >>>>
> >>>> Hm... I did not think about it. But right this can be broken
> >>>> too...
> >>>
> >>> Tony, Javier: are you aware of above ↑↑↑ problem?
> >>>
> >>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> >>> skeleton.dtsi usage") should be really reverted.
> >>
> >> I don't think reverting the mentioned commit is the correct fix
> >> for your problem. We are trying to get rid of skeleton.dtsi for
> >> many reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
> >> skeleton.dtsi").
> >
> > $ git show 3ebee5a2e141
> >
> > * The default empty /chosen and /aliases are somewhat useless...
> >
> > That is not truth, they are not useless as Russell King wrote --
> > removing them break ATAG support.
> >
> > (But that commit is for arm64 which probably is not using ATAGs...
> > But I do not know. At least it is not truth for 32bit arm.)
> >
> >> Also, the chosen node is mentioned to be optional in the ePAPR
> >> document and u-boot creates a chosen node if isn't found [0] so
> >> this issue is only present in boards that don't use u-boot like
> >> the N900/N950/N9 phones.
> >
> > Linux arm decompressor does not propagate ATAGs when /chosen is
> > missing. Sorry, but if for Linux /chosen is required (and without
> > it is broken!) then some ePARP document does not apply there.
> > Either Linux code needs to be fixed (so /chosen will be really
> > only optional) or /chosen stay in Linux required. There is no
> > other option.
> >
> > And I hope that U-boot is not the only one bootloader which Linux
> > kernel supports. I thought that I can use *any* bootloader to boot
> > Linux kernel not just U-Boot which is doing some magic...
> >
> > With this step you are basically going to break booting Linux
> > kernel with all others bootloaders... And personally I really
> > dislike this idea.
> >
> >> So if NOLO doesn't do the same than u-boot and the kernel expects
> >> a chosen node, I suggest to add an empty chosen node in the
> >> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
> >
> > That would fix a problem for N900, N950 and N9. But not for all
> > other ARM devices which bootloader pass some ATAGs.
> >
> > IIRC rule of kernel is not to break compatibility and that commit
> > 008a2ebcd677 really did it.
> >
> > Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just
> > saying that it cause problems which need to be properly fixed. And
> > if fixing them is harder and will take more time, then correct
> > option is to revert 008a2ebcd677 due to breaking support for more
> > devices.
>
> If you think that others boards may have the same issue, then you
> could add an empty chosen node to omap3.dtsi. As I said I think that
> in practice this will only be needed for the machines using NOLO but
> you are right that in theory you could boot them using other
> bootloaders and having an empty node doesn't cause any harm anyway.
Should not be it part of any arm board? IIRC ATAG support is (or was)
not omap3 specified.
> >> [0]:
> >> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c
> >> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
> >>
> >> Best regards,
>
> Best regards,
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 12:48 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 12:48 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote:
> Hello Pali,
>
> On 12/16/2016 09:32 AM, Pali Roh?r wrote:
> > Hi!
> >
> > On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
> >> Hello Pali,
> >>
> >> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
> >>> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
> >>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
> >>>>
> >>>> wrote:
> >>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
> >>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
> >>>>>> usage") broke support for setting cmdline on Nokia N900 via
> >>>>>> CONFIG_CMDLINE.
> >>>>>>
> >>>>>> It is because arm code booted in DT mode parse cmdline only
> >>>>>> via function early_init_dt_scan_chosen() and that function
> >>>>>> does not fill variable boot_command_line when DTB does not
> >>>>>> contain /chosen entry. It is called from function
> >>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
> >>>>>>
> >>>>>> This patch fixes it by explicitly filling boot_command_line in
> >>>>>> function setup_machine_fdt() after calling
> >>>>>> early_init_dt_scan_nodes() in case boot_command_line still
> >>>>>> remains empty.
> >>>>>
> >>>>> This looks like a hack.
> >>>>>
> >>>>> First, the matter of the ATAGs compatibility. The decompressor
> >>>>> relies on there being a pre-existing /chosen node to insert the
> >>>>> command line and other parameters into. If we've dropped it
> >>>>> (by dropping skeleton.dtsi) then we've just regressed more
> >>>>> than just N900 - the decompressor won't be able to merge the
> >>>>> ATAGs into the concatenated FDT.
> >>>>
> >>>> Hm... I did not think about it. But right this can be broken
> >>>> too...
> >>>
> >>> Tony, Javier: are you aware of above ??? problem?
> >>>
> >>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
> >>> skeleton.dtsi usage") should be really reverted.
> >>
> >> I don't think reverting the mentioned commit is the correct fix
> >> for your problem. We are trying to get rid of skeleton.dtsi for
> >> many reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
> >> skeleton.dtsi").
> >
> > $ git show 3ebee5a2e141
> >
> > * The default empty /chosen and /aliases are somewhat useless...
> >
> > That is not truth, they are not useless as Russell King wrote --
> > removing them break ATAG support.
> >
> > (But that commit is for arm64 which probably is not using ATAGs...
> > But I do not know. At least it is not truth for 32bit arm.)
> >
> >> Also, the chosen node is mentioned to be optional in the ePAPR
> >> document and u-boot creates a chosen node if isn't found [0] so
> >> this issue is only present in boards that don't use u-boot like
> >> the N900/N950/N9 phones.
> >
> > Linux arm decompressor does not propagate ATAGs when /chosen is
> > missing. Sorry, but if for Linux /chosen is required (and without
> > it is broken!) then some ePARP document does not apply there.
> > Either Linux code needs to be fixed (so /chosen will be really
> > only optional) or /chosen stay in Linux required. There is no
> > other option.
> >
> > And I hope that U-boot is not the only one bootloader which Linux
> > kernel supports. I thought that I can use *any* bootloader to boot
> > Linux kernel not just U-Boot which is doing some magic...
> >
> > With this step you are basically going to break booting Linux
> > kernel with all others bootloaders... And personally I really
> > dislike this idea.
> >
> >> So if NOLO doesn't do the same than u-boot and the kernel expects
> >> a chosen node, I suggest to add an empty chosen node in the
> >> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
> >
> > That would fix a problem for N900, N950 and N9. But not for all
> > other ARM devices which bootloader pass some ATAGs.
> >
> > IIRC rule of kernel is not to break compatibility and that commit
> > 008a2ebcd677 really did it.
> >
> > Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just
> > saying that it cause problems which need to be properly fixed. And
> > if fixing them is harder and will take more time, then correct
> > option is to revert 008a2ebcd677 due to breaking support for more
> > devices.
>
> If you think that others boards may have the same issue, then you
> could add an empty chosen node to omap3.dtsi. As I said I think that
> in practice this will only be needed for the machines using NOLO but
> you are right that in theory you could boot them using other
> bootloaders and having an empty node doesn't cause any harm anyway.
Should not be it part of any arm board? IIRC ATAG support is (or was)
not omap3 specified.
> >> [0]:
> >> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c
> >> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
> >>
> >> Best regards,
>
> Best regards,
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/debc8ab2/attachment.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 12:48 ` Pali Rohár
@ 2016-12-16 12:53 ` Javier Martinez Canillas
-1 siblings, 0 replies; 44+ messages in thread
From: Javier Martinez Canillas @ 2016-12-16 12:53 UTC (permalink / raw)
To: Pali Rohár
Cc: Tony Lindgren, Russell King - ARM Linux, Arnd Bergmann,
Robin Murphy, Linus Walleij, Ben Dooks, Ivaylo Dimitrov,
Sebastian Reichel, Aaro Koskinen, Pavel Machek, linux-arm-kernel,
linux-kernel
Hello Pali,
On 12/16/2016 09:48 AM, Pali Rohár wrote:
> On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote:
>> Hello Pali,
>>
>> On 12/16/2016 09:32 AM, Pali Rohár wrote:
>>> Hi!
>>>
>>> On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
>>>> Hello Pali,
>>>>
>>>> On 12/16/2016 08:46 AM, Pali Rohár wrote:
>>>>> On Thursday 15 December 2016 01:09:20 Pali Rohár wrote:
>>>>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
>>>>>>
>>>>>> wrote:
>>>>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Rohár wrote:
>>>>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>>>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>>>>>> CONFIG_CMDLINE.
>>>>>>>>
>>>>>>>> It is because arm code booted in DT mode parse cmdline only
>>>>>>>> via function early_init_dt_scan_chosen() and that function
>>>>>>>> does not fill variable boot_command_line when DTB does not
>>>>>>>> contain /chosen entry. It is called from function
>>>>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
>>>>>>>>
>>>>>>>> This patch fixes it by explicitly filling boot_command_line in
>>>>>>>> function setup_machine_fdt() after calling
>>>>>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>>>>>> remains empty.
>>>>>>>
>>>>>>> This looks like a hack.
>>>>>>>
>>>>>>> First, the matter of the ATAGs compatibility. The decompressor
>>>>>>> relies on there being a pre-existing /chosen node to insert the
>>>>>>> command line and other parameters into. If we've dropped it
>>>>>>> (by dropping skeleton.dtsi) then we've just regressed more
>>>>>>> than just N900 - the decompressor won't be able to merge the
>>>>>>> ATAGs into the concatenated FDT.
>>>>>>
>>>>>> Hm... I did not think about it. But right this can be broken
>>>>>> too...
>>>>>
>>>>> Tony, Javier: are you aware of above ↑↑↑ problem?
>>>>>
>>>>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
>>>>> skeleton.dtsi usage") should be really reverted.
>>>>
>>>> I don't think reverting the mentioned commit is the correct fix
>>>> for your problem. We are trying to get rid of skeleton.dtsi for
>>>> many reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
>>>> skeleton.dtsi").
>>>
>>> $ git show 3ebee5a2e141
>>>
>>> * The default empty /chosen and /aliases are somewhat useless...
>>>
>>> That is not truth, they are not useless as Russell King wrote --
>>> removing them break ATAG support.
>>>
>>> (But that commit is for arm64 which probably is not using ATAGs...
>>> But I do not know. At least it is not truth for 32bit arm.)
>>>
>>>> Also, the chosen node is mentioned to be optional in the ePAPR
>>>> document and u-boot creates a chosen node if isn't found [0] so
>>>> this issue is only present in boards that don't use u-boot like
>>>> the N900/N950/N9 phones.
>>>
>>> Linux arm decompressor does not propagate ATAGs when /chosen is
>>> missing. Sorry, but if for Linux /chosen is required (and without
>>> it is broken!) then some ePARP document does not apply there.
>>> Either Linux code needs to be fixed (so /chosen will be really
>>> only optional) or /chosen stay in Linux required. There is no
>>> other option.
>>>
>>> And I hope that U-boot is not the only one bootloader which Linux
>>> kernel supports. I thought that I can use *any* bootloader to boot
>>> Linux kernel not just U-Boot which is doing some magic...
>>>
>>> With this step you are basically going to break booting Linux
>>> kernel with all others bootloaders... And personally I really
>>> dislike this idea.
>>>
>>>> So if NOLO doesn't do the same than u-boot and the kernel expects
>>>> a chosen node, I suggest to add an empty chosen node in the
>>>> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
>>>
>>> That would fix a problem for N900, N950 and N9. But not for all
>>> other ARM devices which bootloader pass some ATAGs.
>>>
>>> IIRC rule of kernel is not to break compatibility and that commit
>>> 008a2ebcd677 really did it.
>>>
>>> Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just
>>> saying that it cause problems which need to be properly fixed. And
>>> if fixing them is harder and will take more time, then correct
>>> option is to revert 008a2ebcd677 due to breaking support for more
>>> devices.
>>
>> If you think that others boards may have the same issue, then you
>> could add an empty chosen node to omap3.dtsi. As I said I think that
>> in practice this will only be needed for the machines using NOLO but
>> you are right that in theory you could boot them using other
>> bootloaders and having an empty node doesn't cause any harm anyway.
>
> Should not be it part of any arm board? IIRC ATAG support is (or was)
> not omap3 specified.
>
Yes, but you were talking about commit 008a2ebcd677 which only removed
skeleton.dtsi usage for OMAP3 boards. The same can be done for other
SoCs in its top level dtsi for the SoC family of course.
>>>> [0]:
>>>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c
>>>> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>>>>
>>>> Best regards,
>>
>> Best regards,
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 44+ messages in thread* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 12:53 ` Javier Martinez Canillas
0 siblings, 0 replies; 44+ messages in thread
From: Javier Martinez Canillas @ 2016-12-16 12:53 UTC (permalink / raw)
To: linux-arm-kernel
Hello Pali,
On 12/16/2016 09:48 AM, Pali Roh?r wrote:
> On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote:
>> Hello Pali,
>>
>> On 12/16/2016 09:32 AM, Pali Roh?r wrote:
>>> Hi!
>>>
>>> On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote:
>>>> Hello Pali,
>>>>
>>>> On 12/16/2016 08:46 AM, Pali Roh?r wrote:
>>>>> On Thursday 15 December 2016 01:09:20 Pali Roh?r wrote:
>>>>>> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux
>>>>>>
>>>>>> wrote:
>>>>>>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh?r wrote:
>>>>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi
>>>>>>>> usage") broke support for setting cmdline on Nokia N900 via
>>>>>>>> CONFIG_CMDLINE.
>>>>>>>>
>>>>>>>> It is because arm code booted in DT mode parse cmdline only
>>>>>>>> via function early_init_dt_scan_chosen() and that function
>>>>>>>> does not fill variable boot_command_line when DTB does not
>>>>>>>> contain /chosen entry. It is called from function
>>>>>>>> early_init_dt_scan_nodes() in setup_machine_fdt().
>>>>>>>>
>>>>>>>> This patch fixes it by explicitly filling boot_command_line in
>>>>>>>> function setup_machine_fdt() after calling
>>>>>>>> early_init_dt_scan_nodes() in case boot_command_line still
>>>>>>>> remains empty.
>>>>>>>
>>>>>>> This looks like a hack.
>>>>>>>
>>>>>>> First, the matter of the ATAGs compatibility. The decompressor
>>>>>>> relies on there being a pre-existing /chosen node to insert the
>>>>>>> command line and other parameters into. If we've dropped it
>>>>>>> (by dropping skeleton.dtsi) then we've just regressed more
>>>>>>> than just N900 - the decompressor won't be able to merge the
>>>>>>> ATAGs into the concatenated FDT.
>>>>>>
>>>>>> Hm... I did not think about it. But right this can be broken
>>>>>> too...
>>>>>
>>>>> Tony, Javier: are you aware of above ??? problem?
>>>>>
>>>>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove
>>>>> skeleton.dtsi usage") should be really reverted.
>>>>
>>>> I don't think reverting the mentioned commit is the correct fix
>>>> for your problem. We are trying to get rid of skeleton.dtsi for
>>>> many reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill
>>>> skeleton.dtsi").
>>>
>>> $ git show 3ebee5a2e141
>>>
>>> * The default empty /chosen and /aliases are somewhat useless...
>>>
>>> That is not truth, they are not useless as Russell King wrote --
>>> removing them break ATAG support.
>>>
>>> (But that commit is for arm64 which probably is not using ATAGs...
>>> But I do not know. At least it is not truth for 32bit arm.)
>>>
>>>> Also, the chosen node is mentioned to be optional in the ePAPR
>>>> document and u-boot creates a chosen node if isn't found [0] so
>>>> this issue is only present in boards that don't use u-boot like
>>>> the N900/N950/N9 phones.
>>>
>>> Linux arm decompressor does not propagate ATAGs when /chosen is
>>> missing. Sorry, but if for Linux /chosen is required (and without
>>> it is broken!) then some ePARP document does not apply there.
>>> Either Linux code needs to be fixed (so /chosen will be really
>>> only optional) or /chosen stay in Linux required. There is no
>>> other option.
>>>
>>> And I hope that U-boot is not the only one bootloader which Linux
>>> kernel supports. I thought that I can use *any* bootloader to boot
>>> Linux kernel not just U-Boot which is doing some magic...
>>>
>>> With this step you are basically going to break booting Linux
>>> kernel with all others bootloaders... And personally I really
>>> dislike this idea.
>>>
>>>> So if NOLO doesn't do the same than u-boot and the kernel expects
>>>> a chosen node, I suggest to add an empty chosen node in the
>>>> omap3-n900.dts and omap3-n950-n9.dtsi device tree source files.
>>>
>>> That would fix a problem for N900, N950 and N9. But not for all
>>> other ARM devices which bootloader pass some ATAGs.
>>>
>>> IIRC rule of kernel is not to break compatibility and that commit
>>> 008a2ebcd677 really did it.
>>>
>>> Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just
>>> saying that it cause problems which need to be properly fixed. And
>>> if fixing them is harder and will take more time, then correct
>>> option is to revert 008a2ebcd677 due to breaking support for more
>>> devices.
>>
>> If you think that others boards may have the same issue, then you
>> could add an empty chosen node to omap3.dtsi. As I said I think that
>> in practice this will only be needed for the machines using NOLO but
>> you are right that in theory you could boot them using other
>> bootloaders and having an empty node doesn't cause any harm anyway.
>
> Should not be it part of any arm board? IIRC ATAG support is (or was)
> not omap3 specified.
>
Yes, but you were talking about commit 008a2ebcd677 which only removed
skeleton.dtsi usage for OMAP3 boards. The same can be done for other
SoCs in its top level dtsi for the SoC family of course.
>>>> [0]:
>>>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c
>>>> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226
>>>>
>>>> Best regards,
>>
>> Best regards,
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 12:53 ` Javier Martinez Canillas
@ 2016-12-16 15:40 ` Tony Lindgren
-1 siblings, 0 replies; 44+ messages in thread
From: Tony Lindgren @ 2016-12-16 15:40 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Pali Rohár, Russell King - ARM Linux, Arnd Bergmann,
Robin Murphy, Linus Walleij, Ben Dooks, Ivaylo Dimitrov,
Sebastian Reichel, Aaro Koskinen, Pavel Machek, linux-arm-kernel,
linux-kernel
* Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> On 12/16/2016 09:48 AM, Pali Rohár wrote:
> >>> saying that it cause problems which need to be properly fixed. And
> >>> if fixing them is harder and will take more time, then correct
> >>> option is to revert 008a2ebcd677 due to breaking support for more
> >>> devices.
> >>
> >> If you think that others boards may have the same issue, then you
> >> could add an empty chosen node to omap3.dtsi. As I said I think that
> >> in practice this will only be needed for the machines using NOLO but
> >> you are right that in theory you could boot them using other
> >> bootloaders and having an empty node doesn't cause any harm anyway.
> >
> > Should not be it part of any arm board? IIRC ATAG support is (or was)
> > not omap3 specified.
> >
>
> Yes, but you were talking about commit 008a2ebcd677 which only removed
> skeleton.dtsi usage for OMAP3 boards. The same can be done for other
> SoCs in its top level dtsi for the SoC family of course.
Yeah probaby best to add the empty chosen node to the ones that had
skeleton.dtsi removed.
And I think the code should print a warning if no chosen node is
found?
Regards,
Tony
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 15:40 ` Tony Lindgren
0 siblings, 0 replies; 44+ messages in thread
From: Tony Lindgren @ 2016-12-16 15:40 UTC (permalink / raw)
To: linux-arm-kernel
* Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> On 12/16/2016 09:48 AM, Pali Roh?r wrote:
> >>> saying that it cause problems which need to be properly fixed. And
> >>> if fixing them is harder and will take more time, then correct
> >>> option is to revert 008a2ebcd677 due to breaking support for more
> >>> devices.
> >>
> >> If you think that others boards may have the same issue, then you
> >> could add an empty chosen node to omap3.dtsi. As I said I think that
> >> in practice this will only be needed for the machines using NOLO but
> >> you are right that in theory you could boot them using other
> >> bootloaders and having an empty node doesn't cause any harm anyway.
> >
> > Should not be it part of any arm board? IIRC ATAG support is (or was)
> > not omap3 specified.
> >
>
> Yes, but you were talking about commit 008a2ebcd677 which only removed
> skeleton.dtsi usage for OMAP3 boards. The same can be done for other
> SoCs in its top level dtsi for the SoC family of course.
Yeah probaby best to add the empty chosen node to the ones that had
skeleton.dtsi removed.
And I think the code should print a warning if no chosen node is
found?
Regards,
Tony
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 15:40 ` Tony Lindgren
@ 2016-12-16 16:13 ` Pali Rohár
-1 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 16:13 UTC (permalink / raw)
To: Tony Lindgren
Cc: Javier Martinez Canillas, Russell King - ARM Linux,
Arnd Bergmann, Robin Murphy, Linus Walleij, Ben Dooks,
Ivaylo Dimitrov, Sebastian Reichel, Aaro Koskinen, Pavel Machek,
linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 1589 bytes --]
On Friday 16 December 2016 16:40:30 Tony Lindgren wrote:
> * Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> > On 12/16/2016 09:48 AM, Pali Rohár wrote:
> > >>> saying that it cause problems which need to be properly fixed.
> > >>> And if fixing them is harder and will take more time, then
> > >>> correct option is to revert 008a2ebcd677 due to breaking
> > >>> support for more devices.
> > >>
> > >> If you think that others boards may have the same issue, then
> > >> you could add an empty chosen node to omap3.dtsi. As I said I
> > >> think that in practice this will only be needed for the
> > >> machines using NOLO but you are right that in theory you could
> > >> boot them using other bootloaders and having an empty node
> > >> doesn't cause any harm anyway.
> > >
> > > Should not be it part of any arm board? IIRC ATAG support is (or
> > > was) not omap3 specified.
> >
> > Yes, but you were talking about commit 008a2ebcd677 which only
> > removed skeleton.dtsi usage for OMAP3 boards. The same can be done
> > for other SoCs in its top level dtsi for the SoC family of course.
>
> Yeah probaby best to add the empty chosen node to the ones that had
> skeleton.dtsi removed.
Ok. But still I think that it should be added globally to all arm board
which can be booted by ATAG bootloader.
> And I think the code should print a warning if no chosen node is
> found?
Which code? Decompressor? Yes, it should but I do not know if at that
time is (serial) console usable...
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 16:13 ` Pali Rohár
0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2016-12-16 16:13 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 16 December 2016 16:40:30 Tony Lindgren wrote:
> * Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> > On 12/16/2016 09:48 AM, Pali Roh?r wrote:
> > >>> saying that it cause problems which need to be properly fixed.
> > >>> And if fixing them is harder and will take more time, then
> > >>> correct option is to revert 008a2ebcd677 due to breaking
> > >>> support for more devices.
> > >>
> > >> If you think that others boards may have the same issue, then
> > >> you could add an empty chosen node to omap3.dtsi. As I said I
> > >> think that in practice this will only be needed for the
> > >> machines using NOLO but you are right that in theory you could
> > >> boot them using other bootloaders and having an empty node
> > >> doesn't cause any harm anyway.
> > >
> > > Should not be it part of any arm board? IIRC ATAG support is (or
> > > was) not omap3 specified.
> >
> > Yes, but you were talking about commit 008a2ebcd677 which only
> > removed skeleton.dtsi usage for OMAP3 boards. The same can be done
> > for other SoCs in its top level dtsi for the SoC family of course.
>
> Yeah probaby best to add the empty chosen node to the ones that had
> skeleton.dtsi removed.
Ok. But still I think that it should be added globally to all arm board
which can be booted by ATAG bootloader.
> And I think the code should print a warning if no chosen node is
> found?
Which code? Decompressor? Yes, it should but I do not know if at that
time is (serial) console usable...
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161216/997b8f16/attachment-0001.sig>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 16:13 ` Pali Rohár
@ 2016-12-16 16:21 ` Tony Lindgren
-1 siblings, 0 replies; 44+ messages in thread
From: Tony Lindgren @ 2016-12-16 16:21 UTC (permalink / raw)
To: Pali Rohár
Cc: Javier Martinez Canillas, Russell King - ARM Linux,
Arnd Bergmann, Robin Murphy, Linus Walleij, Ben Dooks,
Ivaylo Dimitrov, Sebastian Reichel, Aaro Koskinen, Pavel Machek,
linux-arm-kernel, linux-kernel
* Pali Rohár <pali.rohar@gmail.com> [161216 08:14]:
> On Friday 16 December 2016 16:40:30 Tony Lindgren wrote:
> > * Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> > > On 12/16/2016 09:48 AM, Pali Rohár wrote:
> > > >>> saying that it cause problems which need to be properly fixed.
> > > >>> And if fixing them is harder and will take more time, then
> > > >>> correct option is to revert 008a2ebcd677 due to breaking
> > > >>> support for more devices.
> > > >>
> > > >> If you think that others boards may have the same issue, then
> > > >> you could add an empty chosen node to omap3.dtsi. As I said I
> > > >> think that in practice this will only be needed for the
> > > >> machines using NOLO but you are right that in theory you could
> > > >> boot them using other bootloaders and having an empty node
> > > >> doesn't cause any harm anyway.
> > > >
> > > > Should not be it part of any arm board? IIRC ATAG support is (or
> > > > was) not omap3 specified.
> > >
> > > Yes, but you were talking about commit 008a2ebcd677 which only
> > > removed skeleton.dtsi usage for OMAP3 boards. The same can be done
> > > for other SoCs in its top level dtsi for the SoC family of course.
> >
> > Yeah probaby best to add the empty chosen node to the ones that had
> > skeleton.dtsi removed.
>
> Ok. But still I think that it should be added globally to all arm board
> which can be booted by ATAG bootloader.
Hmm you mean by the decompressor?
> > And I think the code should print a warning if no chosen node is
> > found?
>
> Which code? Decompressor? Yes, it should but I do not know if at that
> time is (serial) console usable...
Oh right, never mind..
Tony
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 16:21 ` Tony Lindgren
0 siblings, 0 replies; 44+ messages in thread
From: Tony Lindgren @ 2016-12-16 16:21 UTC (permalink / raw)
To: linux-arm-kernel
* Pali Roh?r <pali.rohar@gmail.com> [161216 08:14]:
> On Friday 16 December 2016 16:40:30 Tony Lindgren wrote:
> > * Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> > > On 12/16/2016 09:48 AM, Pali Roh?r wrote:
> > > >>> saying that it cause problems which need to be properly fixed.
> > > >>> And if fixing them is harder and will take more time, then
> > > >>> correct option is to revert 008a2ebcd677 due to breaking
> > > >>> support for more devices.
> > > >>
> > > >> If you think that others boards may have the same issue, then
> > > >> you could add an empty chosen node to omap3.dtsi. As I said I
> > > >> think that in practice this will only be needed for the
> > > >> machines using NOLO but you are right that in theory you could
> > > >> boot them using other bootloaders and having an empty node
> > > >> doesn't cause any harm anyway.
> > > >
> > > > Should not be it part of any arm board? IIRC ATAG support is (or
> > > > was) not omap3 specified.
> > >
> > > Yes, but you were talking about commit 008a2ebcd677 which only
> > > removed skeleton.dtsi usage for OMAP3 boards. The same can be done
> > > for other SoCs in its top level dtsi for the SoC family of course.
> >
> > Yeah probaby best to add the empty chosen node to the ones that had
> > skeleton.dtsi removed.
>
> Ok. But still I think that it should be added globally to all arm board
> which can be booted by ATAG bootloader.
Hmm you mean by the decompressor?
> > And I think the code should print a warning if no chosen node is
> > found?
>
> Which code? Decompressor? Yes, it should but I do not know if at that
> time is (serial) console usable...
Oh right, never mind..
Tony
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 16:13 ` Pali Rohár
@ 2016-12-16 16:27 ` Russell King - ARM Linux
-1 siblings, 0 replies; 44+ messages in thread
From: Russell King - ARM Linux @ 2016-12-16 16:27 UTC (permalink / raw)
To: Pali Rohár
Cc: Tony Lindgren, Javier Martinez Canillas, Arnd Bergmann,
Robin Murphy, Linus Walleij, Ben Dooks, Ivaylo Dimitrov,
Sebastian Reichel, Aaro Koskinen, Pavel Machek, linux-arm-kernel,
linux-kernel
On Fri, Dec 16, 2016 at 05:13:46PM +0100, Pali Rohár wrote:
> On Friday 16 December 2016 16:40:30 Tony Lindgren wrote:
> > * Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> > > On 12/16/2016 09:48 AM, Pali Rohár wrote:
> > > >>> saying that it cause problems which need to be properly fixed.
> > > >>> And if fixing them is harder and will take more time, then
> > > >>> correct option is to revert 008a2ebcd677 due to breaking
> > > >>> support for more devices.
> > > >>
> > > >> If you think that others boards may have the same issue, then
> > > >> you could add an empty chosen node to omap3.dtsi. As I said I
> > > >> think that in practice this will only be needed for the
> > > >> machines using NOLO but you are right that in theory you could
> > > >> boot them using other bootloaders and having an empty node
> > > >> doesn't cause any harm anyway.
> > > >
> > > > Should not be it part of any arm board? IIRC ATAG support is (or
> > > > was) not omap3 specified.
> > >
> > > Yes, but you were talking about commit 008a2ebcd677 which only
> > > removed skeleton.dtsi usage for OMAP3 boards. The same can be done
> > > for other SoCs in its top level dtsi for the SoC family of course.
> >
> > Yeah probaby best to add the empty chosen node to the ones that had
> > skeleton.dtsi removed.
>
> Ok. But still I think that it should be added globally to all arm board
> which can be booted by ATAG bootloader.
+1
> > And I think the code should print a warning if no chosen node is
> > found?
>
> Which code? Decompressor? Yes, it should but I do not know if at that
> time is (serial) console usable...
It isn't in multiplatform situations. Best if the kernel prints it,
because then it can be logged.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 16:27 ` Russell King - ARM Linux
0 siblings, 0 replies; 44+ messages in thread
From: Russell King - ARM Linux @ 2016-12-16 16:27 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 16, 2016 at 05:13:46PM +0100, Pali Roh?r wrote:
> On Friday 16 December 2016 16:40:30 Tony Lindgren wrote:
> > * Javier Martinez Canillas <javier@osg.samsung.com> [161216 04:54]:
> > > On 12/16/2016 09:48 AM, Pali Roh?r wrote:
> > > >>> saying that it cause problems which need to be properly fixed.
> > > >>> And if fixing them is harder and will take more time, then
> > > >>> correct option is to revert 008a2ebcd677 due to breaking
> > > >>> support for more devices.
> > > >>
> > > >> If you think that others boards may have the same issue, then
> > > >> you could add an empty chosen node to omap3.dtsi. As I said I
> > > >> think that in practice this will only be needed for the
> > > >> machines using NOLO but you are right that in theory you could
> > > >> boot them using other bootloaders and having an empty node
> > > >> doesn't cause any harm anyway.
> > > >
> > > > Should not be it part of any arm board? IIRC ATAG support is (or
> > > > was) not omap3 specified.
> > >
> > > Yes, but you were talking about commit 008a2ebcd677 which only
> > > removed skeleton.dtsi usage for OMAP3 boards. The same can be done
> > > for other SoCs in its top level dtsi for the SoC family of course.
> >
> > Yeah probaby best to add the empty chosen node to the ones that had
> > skeleton.dtsi removed.
>
> Ok. But still I think that it should be added globally to all arm board
> which can be booted by ATAG bootloader.
+1
> > And I think the code should print a warning if no chosen node is
> > found?
>
> Which code? Decompressor? Yes, it should but I do not know if at that
> time is (serial) console usable...
It isn't in multiplatform situations. Best if the kernel prints it,
because then it can be logged.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
2016-12-16 15:40 ` Tony Lindgren
@ 2016-12-16 16:57 ` Mark Rutland
-1 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2016-12-16 16:57 UTC (permalink / raw)
To: Tony Lindgren
Cc: Javier Martinez Canillas, Ivaylo Dimitrov, Pavel Machek,
Arnd Bergmann, Aaro Koskinen, linux-kernel, Linus Walleij,
Russell King - ARM Linux, Sebastian Reichel, Ben Dooks,
Pali Rohár, Robin Murphy, linux-arm-kernel
On Fri, Dec 16, 2016 at 07:40:30AM -0800, Tony Lindgren wrote:
> Yeah probaby best to add the empty chosen node to the ones that had
> skeleton.dtsi removed.
Yes please!
We should probably update the comment in skeleton.dtsi to be more
explicit. I had intended it to mean that chosen should always be
present in the dts, but I worded it poorly.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs
@ 2016-12-16 16:57 ` Mark Rutland
0 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2016-12-16 16:57 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 16, 2016 at 07:40:30AM -0800, Tony Lindgren wrote:
> Yeah probaby best to add the empty chosen node to the ones that had
> skeleton.dtsi removed.
Yes please!
We should probably update the comment in skeleton.dtsi to be more
explicit. I had intended it to mean that chosen should always be
present in the dts, but I worded it poorly.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread