Re: [PATCH] MIPS: implement a "bootargs-append" DT property

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

On Sat, Nov 11, 2017 at 09:19:48AM -0800, Daniel Gimpelevich wrote:
> There are two uses for this:
> 
> 1) It may be useful to split a device-specific kernel command line between
> a .dts file and a .dtsi file, with "bootargs" in one and "bootargs-append"
> in the other, such as for variations of a reference board.
> 
> 2) There are kernel configuration options for prepending "bootargs" to the
> kernel command line that the bootloader has passed, but not for appending.
> A new option for this would be a less future-proof solution, since things
> like this should be in the dtb.
> 
> This is tested on MIPS, but it can be useful on other architectures also.
> 
> Signed-off-by: Daniel Gimpelevich <daniel-R/FLGEdV95bo9U+Z1CfBt0SU0eOFXohjCypLqA8HKkk@public.gmane.org>

The device tree maintainers should be on Cc. Adding them now.

Cheers
James

> ---
>  arch/mips/kernel/setup.c | 3 +++
>  drivers/of/fdt.c         | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index fe39397..95e9bf2 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -826,7 +826,10 @@ static void __init arch_mem_init(char **cmdline_p)
>  	extern void plat_mem_setup(void);
>  
>  	/* call board setup routine */
> +	strlcpy(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
>  	plat_mem_setup();
> +	if (strncmp(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE) == 0)
> +		boot_command_line[0] = '\0';
>  
>  	/*
>  	 * Make sure all kernel memory is in the maps.  The "UP" and
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index ce30c9a..65dbda6 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1127,6 +1127,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	p = of_get_flat_dt_prop(node, "bootargs", &l);
>  	if (p != NULL && l > 0)
>  		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> +	p = of_get_flat_dt_prop(node, "bootargs-append", &l);
> +	if (p != NULL && l > 0)
> +		strlcat(data, p, min_t(int, strlen(data) + l,
> +					COMMAND_LINE_SIZE));
>  
>  	/*
>  	 * CONFIG_CMDLINE is meant to be a default in case nothing else
> -- 
> 1.9.1
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Geert Uytterhoeven]

Hi James, Daniel,

On Mon, Nov 13, 2017 at 12:23 PM, James Hogan <james.hogan@mips.com> wrote:
> On Sat, Nov 11, 2017 at 09:19:48AM -0800, Daniel Gimpelevich wrote:
>> There are two uses for this:
>>
>> 1) It may be useful to split a device-specific kernel command line between
>> a .dts file and a .dtsi file, with "bootargs" in one and "bootargs-append"
>> in the other, such as for variations of a reference board.

I've seen other use cases, e.g. the extension of the du node's "clocks" and
"clock-names" properties from arch/arm64/boot/dts/renesas/r8a7795.dtsi to
arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts.

To avoid the proliferation of "-append" versions of existing properties, what
about handling this in dtc, by adding support for an "/append-property/"
keyword?

     bootargs = "first part"
     ...
     /append-property/ bootargs = " second part".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Daniel Gimpelevich]

On Mon, 2017-11-13 at 13:31 +0100, Geert Uytterhoeven wrote:
> I've seen other use cases, e.g. the extension of the du node's
> "clocks" and
> "clock-names" properties from arch/arm64/boot/dts/renesas/r8a7795.dtsi
> to
> arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts.
> 
> To avoid the proliferation of "-append" versions of existing
> properties, what
> about handling this in dtc, by adding support for an
> "/append-property/"
> keyword?

That would not address use case #2 that I mentioned, where the dtb would
have a "bootargs-append" property but no "bootargs" property.

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Geert Uytterhoeven]

Hi Daniel,

On Mon, Nov 13, 2017 at 1:42 PM, Daniel Gimpelevich
<daniel-R/FLGEdV95bo9U+Z1CfBt0SU0eOFXohjCypLqA8HKkk@public.gmane.org> wrote:
> On Mon, 2017-11-13 at 13:31 +0100, Geert Uytterhoeven wrote:
>> I've seen other use cases, e.g. the extension of the du node's
>> "clocks" and
>> "clock-names" properties from arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> to
>> arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts.
>>
>> To avoid the proliferation of "-append" versions of existing
>> properties, what
>> about handling this in dtc, by adding support for an
>> "/append-property/"
>> keyword?
>
> That would not address use case #2 that I mentioned, where the dtb would
> have a "bootargs-append" property but no "bootargs" property.

For use case #2, you were talking about kernel configuration?
Isn't that something completely different?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Rob Herring]

On Mon, Nov 13, 2017 at 5:23 AM, James Hogan <james.hogan-8NJIiSa5LzA@public.gmane.org> wrote:
> On Sat, Nov 11, 2017 at 09:19:48AM -0800, Daniel Gimpelevich wrote:
>> There are two uses for this:
>>
>> 1) It may be useful to split a device-specific kernel command line between
>> a .dts file and a .dtsi file, with "bootargs" in one and "bootargs-append"
>> in the other, such as for variations of a reference board.

What Geert said.

>> 2) There are kernel configuration options for prepending "bootargs" to the
>> kernel command line that the bootloader has passed, but not for appending.
>> A new option for this would be a less future-proof solution, since things
>> like this should be in the dtb.

This is a kernel problem. What's the use case where you want the DT to
override the kernel?

One way you could handle this is make bootargs be multiple strings.
Well 2 specifically, the first string is prepended and the 2nd is
appended. That complicates how you'd implement /append-property/
though as you'd probably want to support both string cat and 2
strings. Though the latter works more generically without knowing the
data type.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Daniel Gimpelevich]

On Mon, 2017-11-13 at 10:34 -0600, Rob Herring wrote:
> This is a kernel problem. What's the use case where you want the DT to
> override the kernel?
> 
> One way you could handle this is make bootargs be multiple strings.
> Well 2 specifically, the first string is prepended and the 2nd is
> appended. That complicates how you'd implement /append-property/
> though as you'd probably want to support both string cat and 2
> strings. Though the latter works more generically without knowing the
> data type.

Let's say the bootloader tells the kernel that the command line is "foo
bar" and that "baz" is in the dtb. Currently, there are kernel
configuration options to control whether this means the kernel's command
line will be "baz" or "baz foo bar" instead, but there is no way to turn
it into "foo bar baz" without either creating new kernel configuration
options or this patch. Implementing it via this patch would allow a
theoretical distro to use a generic kernel across different devices that
need the arguments from their bootloader manipulated in different ways.

Ideally, the MIPS-specific kernel configuration options for how to treat
the dtb's bootargs with respect to the bootloader's bootargs should go
away in favor of an analogous device tree property "bootargs-prepend"
for this reason. The kernel configuration options to supply a hard-coded
default command line if the bootloader does not supply one, and to
override what the bootloader and dtb supply, would remain. This would
separate the need for an alternate or "recovery" kernel to supply its
own bootargs to override the dtb from the need to have device-specific
bootargs in the dtb override a legacy bootloader, where the manner its
bootargs are overridden may be device-specific.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Rob Herring]

On Mon, Nov 13, 2017 at 9:05 PM, Daniel Gimpelevich
<daniel-R/FLGEdV95bo9U+Z1CfBt0SU0eOFXohjCypLqA8HKkk@public.gmane.org> wrote:
> On Mon, 2017-11-13 at 10:34 -0600, Rob Herring wrote:
>> This is a kernel problem. What's the use case where you want the DT to
>> override the kernel?
>>
>> One way you could handle this is make bootargs be multiple strings.
>> Well 2 specifically, the first string is prepended and the 2nd is
>> appended. That complicates how you'd implement /append-property/
>> though as you'd probably want to support both string cat and 2
>> strings. Though the latter works more generically without knowing the
>> data type.
>
> Let's say the bootloader tells the kernel that the command line is "foo
> bar" and that "baz" is in the dtb. Currently, there are kernel
> configuration options to control whether this means the kernel's command
> line will be "baz" or "baz foo bar" instead, but there is no way to turn
> it into "foo bar baz" without either creating new kernel configuration
> options or this patch. Implementing it via this patch would allow a
> theoretical distro to use a generic kernel across different devices that
> need the arguments from their bootloader manipulated in different ways.

I understand you can't apply random strings in any order you like. I'm
questioning the need to do that and what are the concrete example(s)
where you need that ability.

I'd generally expect that only board specific options are in the dtb,
and a distro will add it's own options common for all and the specific
arch into the bootloader config files. And generally, the last thing
loaded gets the last say in what is set.

> Ideally, the MIPS-specific kernel configuration options for how to treat
> the dtb's bootargs with respect to the bootloader's bootargs should go
> away in favor of an analogous device tree property "bootargs-prepend"
> for this reason. The kernel configuration options to supply a hard-coded
> default command line if the bootloader does not supply one, and to
> override what the bootloader and dtb supply, would remain. This would
> separate the need for an alternate or "recovery" kernel to supply its
> own bootargs to override the dtb from the need to have device-specific
> bootargs in the dtb override a legacy bootloader, where the manner its
> bootargs are overridden may be device-specific.

What h/w specific options would be needed for recovery kernel? That's
getting into putting not just Linux specifics into the dtb, but distro
specifics there. While yes, the dts files often already have bootargs
filled with Linux options, the intention is really that the bootloader
fills in bootargs. And if you have multiple kernels or OSs, then the
bootloader provides the mechanism to choose and boot with the right
options.

I think the kernel (being last) should fully decide what to do:
append, prepend, and/or override. There was some work a while back to
support more flexible command line handling and be arch neutral, but
it never got merged.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Daniel Gimpelevich]

On Tue, 2017-11-14 at 11:18 -0600, Rob Herring wrote:
> I understand you can't apply random strings in any order you like. I'm
> questioning the need to do that and what are the concrete example(s)
> where you need that ability.

Concrete:
Pre-DT bootloader provides "console=/dev/someunusualdevice" and
"root=/dev/mtdblock1" in argv. The dtb arranges flash differently and
supplies "root=/dev/mtd2" in bootargs. Kernel uses /dev/mtdblock1 and
panics, or alternatively uses /dev/mtd2 with no console.

> I'd generally expect that only board specific options are in the dtb,
> and a distro will add it's own options common for all and the specific
> arch into the bootloader config files. And generally, the last thing
> loaded gets the last say in what is set.

Bootloaders and their configs are not distro-dependent.
> 
> What h/w specific options would be needed for recovery kernel? That's
> getting into putting not just Linux specifics into the dtb, but distro
> specifics there. While yes, the dts files often already have bootargs
> filled with Linux options, the intention is really that the bootloader
> fills in bootargs. And if you have multiple kernels or OSs, then the
> bootloader provides the mechanism to choose and boot with the right
> options.

Kernels _are_ distro-specific. A recovery kernel might also have options
that are not hardware specific in addition to options that are.

> I think the kernel (being last) should fully decide what to do:
> append, prepend, and/or override. There was some work a while back to
> support more flexible command line handling and be arch neutral, but
> it never got merged.

The kernel would not always have all the information needed for this
decision now like it would before DT. The dtb should decide what
precedence its bootargs have, and the kernel should decide what
precedence compiled-in bootargs have. Such logic is provided by this
patch, but not completely, because there is still no "bootargs-prepend"
property. The only use case I can think of for "bootargs-prepend" would
be to provide a default command line that is specific to a board that
would sometimes have one bootloader, and sometimes another. Currently,
such a situation can be worked around using a board-specific kernel, but
this mechanism is a relic from pre-DT days.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MIPS: implement a "bootargs-append" DT property

[Rob Herring]

On Wed, Nov 15, 2017 at 7:46 PM, Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us> wrote:
> On Tue, 2017-11-14 at 11:18 -0600, Rob Herring wrote:
>> I understand you can't apply random strings in any order you like. I'm
>> questioning the need to do that and what are the concrete example(s)
>> where you need that ability.
>
> Concrete:
> Pre-DT bootloader provides "console=/dev/someunusualdevice" and
> "root=/dev/mtdblock1" in argv. The dtb arranges flash differently and
> supplies "root=/dev/mtd2" in bootargs. Kernel uses /dev/mtdblock1 and
> panics, or alternatively uses /dev/mtd2 with no console.
>
>> I'd generally expect that only board specific options are in the dtb,
>> and a distro will add it's own options common for all and the specific
>> arch into the bootloader config files. And generally, the last thing
>> loaded gets the last say in what is set.
>
> Bootloaders and their configs are not distro-dependent.

For the ones that install grub, they are.

>> What h/w specific options would be needed for recovery kernel? That's
>> getting into putting not just Linux specifics into the dtb, but distro
>> specifics there. While yes, the dts files often already have bootargs
>> filled with Linux options, the intention is really that the bootloader
>> fills in bootargs. And if you have multiple kernels or OSs, then the
>> bootloader provides the mechanism to choose and boot with the right
>> options.
>
> Kernels _are_ distro-specific. A recovery kernel might also have options
> that are not hardware specific in addition to options that are.
>
>> I think the kernel (being last) should fully decide what to do:
>> append, prepend, and/or override. There was some work a while back to
>> support more flexible command line handling and be arch neutral, but
>> it never got merged.
>
> The kernel would not always have all the information needed for this
> decision now like it would before DT. The dtb should decide what
> precedence its bootargs have, and the kernel should decide what
> precedence compiled-in bootargs have. Such logic is provided by this
> patch, but not completely, because there is still no "bootargs-prepend"
> property. The only use case I can think of for "bootargs-prepend" would
> be to provide a default command line that is specific to a board that
> would sometimes have one bootloader, and sometimes another. Currently,
> such a situation can be worked around using a board-specific kernel, but
> this mechanism is a relic from pre-DT days.
>