Re: [PATCH] of: support passing console options with stdout-path
> > More worrying, barebox does not support the : either. So there is a > > danger your bootloader suddenly goes silent after a dt blob update. > > Is barebox parsing the property? Or updating it? Or both? I've got no > problem with this patch, but we can also support things like the > 'current-speed' property in the UART to set a default when the stdout > path doesn't have any arguments. Hi Grant I'm no export with barebox. So i just grep'ed the code. It parses the property. Also as with linux, of_find_node_by_path() does not stop at : as you say it should. There is one case of the property being updated, but that is only for the PPC mpc85xx. Barebox does not appear to use 'current-speed' at all. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 2014-11-25 at 15:39 +, Grant Likely wrote: > On Tue, Nov 25, 2014 at 3:24 PM, Ian Campbell wrote: > >> sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\) > > > > Coccinelle rules for this sort of transformation... > > /me still hasn't gotten his head around how to use Coccinelle properly. :-( Me neither, but for the "add an extra NULL parameter" case I can generally find something to cargo cult from ;-) (it's been ages since I had a need though) Ian. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 3:24 PM, Ian Campbell wrote:
> On Tue, 2014-11-25 at 15:20 +, Grant Likely wrote:
>> On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm
>> wrote:
>> > On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
>> >> > + len = strchrnul(path, ':') - path;
>> >> > +
>> >> > for_each_property_of_node(of_aliases, pp) {
>> >> > if (strlen(pp->name) == len && !strncmp(pp->name,
>> >> > path, len)) {
>> >> > np = of_find_node_by_path(pp->value);
>> >>
>> >> Can you add a testcase to drivers/of/unittests.c for this new path
>> >> parsing? Then we know it's correct!
>> >
>> > Will do.
>> >
>> >> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size,
>> >> > u64 align))
>> >> > name = of_get_property(of_chosen,
>> >> > "linux,stdout-path", NULL);
>> >> > if (IS_ENABLED(CONFIG_PPC) && !name)
>> >> > name = of_get_property(of_aliases, "stdout", NULL);
>> >> > - if (name)
>> >> > + if (name) {
>> >> > of_stdout = of_find_node_by_path(name);
>> >> > + of_stdout_options = strchr(name, ':');
>> >> > + if (of_stdout_options != NULL)
>> >> > + of_stdout_options++;
>> >> > + }
>> >>
>> >> Not so good. The ':' should actually be a generic part of
>> >> of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
>> >> There are other places that would use it. I would add a second,
>> >> optional, argument to of_find_node_by_path() that will update a pointer
>> >> to the arguments after the ':'.
>> >
>> > So, I agree this would be nicer...
>> >
>> > However, I'm counting 157 callers of this function outside of
>> > drivers/of and 43 inside. Any chance you'd let me get away with a
>> > separate of_find_extra_options_by_path()?
>>
>> I'd rather not. It is a trivial change and can be pretty much done
>> mechanically. Something like (I think; I always have to look up the
>> sed syntax):
>>
>> sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
>
> Coccinelle rules for this sort of transformation...
/me still hasn't gotten his head around how to use Coccinelle properly. :-(
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 2014-11-25 at 15:20 +, Grant Likely wrote:
> On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm
> wrote:
> > On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
> >> > + len = strchrnul(path, ':') - path;
> >> > +
> >> > for_each_property_of_node(of_aliases, pp) {
> >> > if (strlen(pp->name) == len && !strncmp(pp->name,
> >> > path, len)) {
> >> > np = of_find_node_by_path(pp->value);
> >>
> >> Can you add a testcase to drivers/of/unittests.c for this new path
> >> parsing? Then we know it's correct!
> >
> > Will do.
> >
> >> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size,
> >> > u64 align))
> >> > name = of_get_property(of_chosen,
> >> > "linux,stdout-path", NULL);
> >> > if (IS_ENABLED(CONFIG_PPC) && !name)
> >> > name = of_get_property(of_aliases, "stdout", NULL);
> >> > - if (name)
> >> > + if (name) {
> >> > of_stdout = of_find_node_by_path(name);
> >> > + of_stdout_options = strchr(name, ':');
> >> > + if (of_stdout_options != NULL)
> >> > + of_stdout_options++;
> >> > + }
> >>
> >> Not so good. The ':' should actually be a generic part of
> >> of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
> >> There are other places that would use it. I would add a second,
> >> optional, argument to of_find_node_by_path() that will update a pointer
> >> to the arguments after the ':'.
> >
> > So, I agree this would be nicer...
> >
> > However, I'm counting 157 callers of this function outside of
> > drivers/of and 43 inside. Any chance you'd let me get away with a
> > separate of_find_extra_options_by_path()?
>
> I'd rather not. It is a trivial change and can be pretty much done
> mechanically. Something like (I think; I always have to look up the
> sed syntax):
>
> sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
Coccinelle rules for this sort of transformation...
Ian.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm wrote:
> On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
>> > + len = strchrnul(path, ':') - path;
>> > +
>> > for_each_property_of_node(of_aliases, pp) {
>> > if (strlen(pp->name) == len && !strncmp(pp->name,
>> > path, len)) {
>> > np = of_find_node_by_path(pp->value);
>>
>> Can you add a testcase to drivers/of/unittests.c for this new path
>> parsing? Then we know it's correct!
>
> Will do.
>
>> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
>> > align))
>> > name = of_get_property(of_chosen, "linux,stdout-path",
>> > NULL);
>> > if (IS_ENABLED(CONFIG_PPC) && !name)
>> > name = of_get_property(of_aliases, "stdout", NULL);
>> > - if (name)
>> > + if (name) {
>> > of_stdout = of_find_node_by_path(name);
>> > + of_stdout_options = strchr(name, ':');
>> > + if (of_stdout_options != NULL)
>> > + of_stdout_options++;
>> > + }
>>
>> Not so good. The ':' should actually be a generic part of
>> of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
>> There are other places that would use it. I would add a second,
>> optional, argument to of_find_node_by_path() that will update a pointer
>> to the arguments after the ':'.
>
> So, I agree this would be nicer...
>
> However, I'm counting 157 callers of this function outside of
> drivers/of and 43 inside. Any chance you'd let me get away with a
> separate of_find_extra_options_by_path()?
I'd rather not. It is a trivial change and can be pretty much done
mechanically. Something like (I think; I always have to look up the
sed syntax):
sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
If it turns out to be really painful, then I'll relent, but I'd like
you to at least try! :-)
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
> > + len = strchrnul(path, ':') - path;
> > +
> > for_each_property_of_node(of_aliases, pp) {
> > if (strlen(pp->name) == len && !strncmp(pp->name, path,
> > len)) {
> > np = of_find_node_by_path(pp->value);
>
> Can you add a testcase to drivers/of/unittests.c for this new path
> parsing? Then we know it's correct!
Will do.
> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
> > align))
> > name = of_get_property(of_chosen, "linux,stdout-path",
> > NULL);
> > if (IS_ENABLED(CONFIG_PPC) && !name)
> > name = of_get_property(of_aliases, "stdout", NULL);
> > - if (name)
> > + if (name) {
> > of_stdout = of_find_node_by_path(name);
> > + of_stdout_options = strchr(name, ':');
> > + if (of_stdout_options != NULL)
> > + of_stdout_options++;
> > + }
>
> Not so good. The ':' should actually be a generic part of
> of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
> There are other places that would use it. I would add a second,
> optional, argument to of_find_node_by_path() that will update a pointer
> to the arguments after the ':'.
So, I agree this would be nicer...
However, I'm counting 157 callers of this function outside of
drivers/of and 43 inside. Any chance you'd let me get away with a
separate of_find_extra_options_by_path()?
/
Leif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 25 Nov 2014 12:07:29 +
, Ian Campbell
wrote:
> On Tue, 2014-11-25 at 11:17 +, Leif Lindholm wrote:
> > On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote:
> > > On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
> > > > Support specifying console options (like with console=ttyXN,)
> > > > by appending them to the stdout-path property after a separating ':'.
> > > >
> > > > Example:
> > > > stdout-path = "uart0:115200";
> > >
> > > I would very much like to be able to use this -- it will allow
> > > distributions to boot on a board without having to know _anything_ about
> > > the console UART until userspace is up, which would make it possible to
> > > have a completely generic installer image, without requiring the
> > > platform to provide bootargs.
> > >
> > > My only concern is that this conflates the set of kernel command line
> > > options for the UART wit the DT binding for it. I don't know how good
> > > people are at keeping those options stable, and I know they are
> > > currently not documented -- we would need to add a stdout-path options
> > > section to relevant UART bindings.
> >
> > I don't disagree.
> >
> > Current options are fairly well defined and stable, at least for any
> > driver that uses uart_parse_options() (documented in
> > Documentation/serial/driver).
>
> My concern is that this is Linux specific, other OSes may have different
> ideas about how stdout options should be formatted within this property.
> (At least I don't know of any standardisation of the 115200n8 thing --
> I'd love to be corrected!)
>
> If I were a firmware author I'd be wary of specifying a stdout-path with
> a Linux specific suffix.
>
> Search ePAPR for baud it seems that the generic serial binding includes
> a current-speed property in 6.2.1.2. It then goes on a bit ambiguously
> to talk about the NS16550 in 6.2.2 but I think 6.2.1.2 was intended to
> be generic. No mention of stop-bits/parity etc though, they are assumed
> to be set already I think
>
> One thought I had was to define a dt-stdout "pseudo-console" so that
> console=dt-stdout,115200n8 or something could be used.
>
> Anyway I applied your patch to v3.18-rc5 and ran it on a Mustang and it
> didn't work for some reason. I'm using:
>
> fdt set /chosen stdout-path "/soc/serial@1c02:115200"
> setenv bootargs "earlycon=uart8250,mmio32,0x1c02 root=/dev/sda3
> rw debug"
>
> So I get earlycon but then no proper console. Removing earlycon just
> makes that stop working.
>
> With:
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 89c6b33..5dc1718 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1840,6 +1840,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
> align))
> of_stdout_options = strchr(name, ':');
> if (of_stdout_options != NULL)
> of_stdout_options++;
> + printk(KERN_CRIT "%s: name=%s of_stdout=%p
> options=%s\n",
> +__func__, name, of_stdout, of_stdout_options);
> }
> }
>
>
> I can see in dmesg:
> [0.00] of_alias_scan: name=/soc/serial@1c02:115200 of_stdout=
> (null) options=115200
>
> So it seems like of_find_node_by_path() is confused by the ":".
That would be a bug in of_find_node_by_path(). Should add a test case
for that because ':' is supposed to terminate path parsing.
> I've not tried it but I'd have expected something more like:
> if (name) {
> of_stdout_options = strchr(name, ':');
> if (of_stdout_options != NULL) {
> *of_stdout_options = '0';
> of_stdout_options++;
> }
> of_stdout = of_find_node_by_path(name);
> }
This isn't really right because it modifies the property data which is
supposed to be const. of_find_node_by_path() already does the search
piecewise, so it should be fairly straightforward to add a test for the
':'. Anyone care to take a look? If I need to do it then it will take a couple
of weeks before I've got some free time. :-/
>
> i.e. strip the options then do the patch lookup. name is const so this
> won't actually work, but something like it...
>
> Ian.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Mon, 24 Nov 2014 22:23:58 +
, Leif Lindholm
wrote:
> Support specifying console options (like with console=ttyXN,)
> by appending them to the stdout-path property after a separating ':'.
>
> Example:
> stdout-path = "uart0:115200";
>
> This patch also modifies of_find_node_by_path() to match only the
> portion of the path before a ':'.
>
> Signed-off-by: Leif Lindholm
> ---
> drivers/of/base.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 3823edf..89c6b33 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
> struct device_node *of_chosen;
> struct device_node *of_aliases;
> struct device_node *of_stdout;
> +static char *of_stdout_options;
>
> struct kset *of_kset;
>
> @@ -703,6 +704,8 @@ static struct device_node *__of_find_node_by_path(struct
> device_node *parent,
> if (!len)
> return NULL;
>
> + len = strchrnul(path, ':') - path;
> +
> __for_each_child_of_node(parent, child) {
> const char *name = strrchr(child->full_name, '/');
> if (WARN(!name, "malformed device_node %s\n", child->full_name))
> @@ -747,6 +750,8 @@ struct device_node *of_find_node_by_path(const char *path)
> if (!of_aliases)
> return NULL;
>
> + len = strchrnul(path, ':') - path;
> +
> for_each_property_of_node(of_aliases, pp) {
> if (strlen(pp->name) == len && !strncmp(pp->name, path,
> len)) {
> np = of_find_node_by_path(pp->value);
Can you add a testcase to drivers/of/unittests.c for this new path
parsing? Then we know it's correct!
> @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
> align))
> name = of_get_property(of_chosen, "linux,stdout-path",
> NULL);
> if (IS_ENABLED(CONFIG_PPC) && !name)
> name = of_get_property(of_aliases, "stdout", NULL);
> - if (name)
> + if (name) {
> of_stdout = of_find_node_by_path(name);
> + of_stdout_options = strchr(name, ':');
> + if (of_stdout_options != NULL)
> + of_stdout_options++;
> + }
Not so good. The ':' should actually be a generic part of
of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
There are other places that would use it. I would add a second,
optional, argument to of_find_node_by_path() that will update a pointer
to the arguments after the ':'.
g.
> }
>
> if (!of_aliases)
> @@ -1957,7 +1966,7 @@ bool of_console_check(struct device_node *dn, char
> *name, int index)
> {
> if (!dn || dn != of_stdout || console_set_on_cmdline)
> return false;
> - return !add_preferred_console(name, index, NULL);
> + return !add_preferred_console(name, index, of_stdout_options);
> }
> EXPORT_SYMBOL_GPL(of_console_check);
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 25 Nov 2014 00:00:16 +0100 , Andrew Lunn wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: > > Support specifying console options (like with console=ttyXN,) > > by appending them to the stdout-path property after a separating ':'. > > > > Example: > > stdout-path = "uart0:115200"; > > > > This patch also modifies of_find_node_by_path() to match only the > > portion of the path before a ':'. > > Hi Leif > > These appears to somewhat conform to ePAPR, which says: > > A string that specifies the full path to the node representing > the device to be used for boot console output. If the character > ":" is present in the value it terminates the path. > > So you can put any random junk after the :. However, are we going to > have backward/forward compatibility problems, and problems with > bootloaders? The current kernel code does not look for the :. So a new > DT blob on an old kernel will not work so well. For DT, we've generally not been worrying about compatibility in that direction. If we've got a new firmware, we should be able to get a new kernel. Nobody has really screamed bout it yet. > More worrying, barebox does not support the : either. So there is a > danger your bootloader suddenly goes silent after a dt blob update. Is barebox parsing the property? Or updating it? Or both? I've got no problem with this patch, but we can also support things like the 'current-speed' property in the UART to set a default when the stdout path doesn't have any arguments. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 12:07:29PM +, Ian Campbell wrote:
> My concern is that this is Linux specific, other OSes may have different
> ideas about how stdout options should be formatted within this property.
> (At least I don't know of any standardisation of the 115200n8 thing --
> I'd love to be corrected!)
>
> If I were a firmware author I'd be wary of specifying a stdout-path with
> a Linux specific suffix.
> Search ePAPR for baud it seems that the generic serial binding includes
> a current-speed property in 6.2.1.2. It then goes on a bit ambiguously
> to talk about the NS16550 in 6.2.2 but I think 6.2.1.2 was intended to
> be generic. No mention of stop-bits/parity etc though, they are assumed
> to be set already I think
>
> One thought I had was to define a dt-stdout "pseudo-console" so that
> console=dt-stdout,115200n8 or something could be used.
I'll wait for others to comment on the above.
> Anyway I applied your patch to v3.18-rc5 and ran it on a Mustang and it
> didn't work for some reason. I'm using:
>
> fdt set /chosen stdout-path "/soc/serial@1c02:115200"
> setenv bootargs "earlycon=uart8250,mmio32,0x1c02 root=/dev/sda3
> rw debug"
>
> So I get earlycon but then no proper console. Removing earlycon just
> makes that stop working.
Right, so having debugged this a bit offline, I'm amazed I booted
anything at all, given how badly I broke path scanning...
Please ignore previous version - a fixed one follows:
/
Leif
>From aef87fd958902afe881720286d525e10997462b8 Mon Sep 17 00:00:00 2001
From: Leif Lindholm
Date: Mon, 24 Nov 2014 22:23:58 +
Subject: [PATCH] of: support passing console options with stdout-path
Support specifying console options (like with console=ttyXN,)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = "uart0:115200";
This patch also modifies of_find_node_by_path() to match only the
portion of the path before a ':'.
Signed-off-by: Leif Lindholm
---
drivers/of/base.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3823edf..ecd6290 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
struct device_node *of_chosen;
struct device_node *of_aliases;
struct device_node *of_stdout;
+static char *of_stdout_options;
struct kset *of_kset;
@@ -699,10 +700,15 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
{
struct device_node *child;
int len = strchrnul(path, '/') - path;
+ int term;
if (!len)
return NULL;
+ term = strchrnul(path, ':') - path;
+ if (term < len)
+ len = term;
+
__for_each_child_of_node(parent, child) {
const char *name = strrchr(child->full_name, '/');
if (WARN(!name, "malformed device_node %s\n", child->full_name))
@@ -742,11 +748,16 @@ struct device_node *of_find_node_by_path(const char *path)
if (*path != '/') {
char *p = strchrnul(path, '/');
int len = p - path;
+ int term;
/* of_aliases must not be NULL */
if (!of_aliases)
return NULL;
+ term = strchrnul(path, ':') - path;
+ if (term < len)
+ len = term;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp->name) == len && !strncmp(pp->name, path,
len)) {
np = of_find_node_by_path(pp->value);
@@ -1830,8 +1841,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, "linux,stdout-path",
NULL);
if (IS_ENABLED(CONFIG_PPC) && !name)
name = of_get_property(of_aliases, "stdout", NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
}
if (!of_aliases)
@@ -1957,7 +1972,7 @@ bool of_console_check(struct device_node *dn, char *name,
int index)
{
if (!dn || dn != of_stdout || console_set_on_cmdline)
return false;
- return !add_preferred_console(name, index, NULL);
+ return !add_preferred_console(name, index, of_stdout_options);
}
EXPORT_SYMBOL_GPL(of_console_check);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote: > Hi Leif, > > On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: > > Support specifying console options (like with console=ttyXN,) > > by appending them to the stdout-path property after a separating ':'. > > > > Example: > > stdout-path = "uart0:115200"; > > I would very much like to be able to use this -- it will allow > distributions to boot on a board without having to know _anything_ about > the console UART until userspace is up, which would make it possible to > have a completely generic installer image, without requiring the > platform to provide bootargs. Note there are several serial drivers that read back the console options from the hardware assuming it has been initialized correctly by the bootloader already. So when your bootloader already made some output on the console (which is likely) you can do that already. Of course this doesn't solve all problems, like if you want to use a different console for the kernel than for the bootloader. Anyway, this behaviour is very nice since the kernel always uses the same speed for the console as the bootloader does. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 2014-11-25 at 11:17 +, Leif Lindholm wrote:
> On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote:
> > On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
> > > Support specifying console options (like with console=ttyXN,)
> > > by appending them to the stdout-path property after a separating ':'.
> > >
> > > Example:
> > > stdout-path = "uart0:115200";
> >
> > I would very much like to be able to use this -- it will allow
> > distributions to boot on a board without having to know _anything_ about
> > the console UART until userspace is up, which would make it possible to
> > have a completely generic installer image, without requiring the
> > platform to provide bootargs.
> >
> > My only concern is that this conflates the set of kernel command line
> > options for the UART wit the DT binding for it. I don't know how good
> > people are at keeping those options stable, and I know they are
> > currently not documented -- we would need to add a stdout-path options
> > section to relevant UART bindings.
>
> I don't disagree.
>
> Current options are fairly well defined and stable, at least for any
> driver that uses uart_parse_options() (documented in
> Documentation/serial/driver).
My concern is that this is Linux specific, other OSes may have different
ideas about how stdout options should be formatted within this property.
(At least I don't know of any standardisation of the 115200n8 thing --
I'd love to be corrected!)
If I were a firmware author I'd be wary of specifying a stdout-path with
a Linux specific suffix.
Search ePAPR for baud it seems that the generic serial binding includes
a current-speed property in 6.2.1.2. It then goes on a bit ambiguously
to talk about the NS16550 in 6.2.2 but I think 6.2.1.2 was intended to
be generic. No mention of stop-bits/parity etc though, they are assumed
to be set already I think
One thought I had was to define a dt-stdout "pseudo-console" so that
console=dt-stdout,115200n8 or something could be used.
Anyway I applied your patch to v3.18-rc5 and ran it on a Mustang and it
didn't work for some reason. I'm using:
fdt set /chosen stdout-path "/soc/serial@1c02:115200"
setenv bootargs "earlycon=uart8250,mmio32,0x1c02 root=/dev/sda3 rw
debug"
So I get earlycon but then no proper console. Removing earlycon just
makes that stop working.
With:
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 89c6b33..5dc1718 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1840,6 +1840,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
of_stdout_options = strchr(name, ':');
if (of_stdout_options != NULL)
of_stdout_options++;
+ printk(KERN_CRIT "%s: name=%s of_stdout=%p
options=%s\n",
+ __func__, name, of_stdout, of_stdout_options);
}
}
I can see in dmesg:
[0.00] of_alias_scan: name=/soc/serial@1c02:115200 of_stdout=
(null) options=115200
So it seems like of_find_node_by_path() is confused by the ":".
I've not tried it but I'd have expected something more like:
if (name) {
of_stdout_options = strchr(name, ':');
if (of_stdout_options != NULL) {
*of_stdout_options = '0';
of_stdout_options++;
}
of_stdout = of_find_node_by_path(name);
}
i.e. strip the options then do the patch lookup. name is const so this
won't actually work, but something like it...
Ian.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote:
> On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
> > Support specifying console options (like with console=ttyXN,)
> > by appending them to the stdout-path property after a separating ':'.
> >
> > Example:
> > stdout-path = "uart0:115200";
>
> I would very much like to be able to use this -- it will allow
> distributions to boot on a board without having to know _anything_ about
> the console UART until userspace is up, which would make it possible to
> have a completely generic installer image, without requiring the
> platform to provide bootargs.
>
> My only concern is that this conflates the set of kernel command line
> options for the UART wit the DT binding for it. I don't know how good
> people are at keeping those options stable, and I know they are
> currently not documented -- we would need to add a stdout-path options
> section to relevant UART bindings.
I don't disagree.
Current options are fairly well defined and stable, at least for any
driver that uses uart_parse_options() (documented in
Documentation/serial/driver).
>From drivers/tty/serial/serial_core.c:
* uart_parse_options decodes a string containing the serial console
* options. The format of the string is ,
* eg: 115200n8r
/
Leif
>
> >
> > This patch also modifies of_find_node_by_path() to match only the
> > portion of the path before a ':'.
> >
> > Signed-off-by: Leif Lindholm
> > ---
> > drivers/of/base.c | 13 +++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 3823edf..89c6b33 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
> > struct device_node *of_chosen;
> > struct device_node *of_aliases;
> > struct device_node *of_stdout;
> > +static char *of_stdout_options;
> >
> > struct kset *of_kset;
> >
> > @@ -703,6 +704,8 @@ static struct device_node
> > *__of_find_node_by_path(struct device_node *parent,
> > if (!len)
> > return NULL;
> >
> > + len = strchrnul(path, ':') - path;
> > +
> > __for_each_child_of_node(parent, child) {
> > const char *name = strrchr(child->full_name, '/');
> > if (WARN(!name, "malformed device_node %s\n", child->full_name))
> > @@ -747,6 +750,8 @@ struct device_node *of_find_node_by_path(const char
> > *path)
> > if (!of_aliases)
> > return NULL;
> >
> > + len = strchrnul(path, ':') - path;
> > +
> > for_each_property_of_node(of_aliases, pp) {
> > if (strlen(pp->name) == len && !strncmp(pp->name, path,
> > len)) {
> > np = of_find_node_by_path(pp->value);
> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
> > align))
> > name = of_get_property(of_chosen, "linux,stdout-path",
> > NULL);
> > if (IS_ENABLED(CONFIG_PPC) && !name)
> > name = of_get_property(of_aliases, "stdout", NULL);
> > - if (name)
> > + if (name) {
> > of_stdout = of_find_node_by_path(name);
> > + of_stdout_options = strchr(name, ':');
> > + if (of_stdout_options != NULL)
> > + of_stdout_options++;
> > + }
> > }
> >
> > if (!of_aliases)
> > @@ -1957,7 +1966,7 @@ bool of_console_check(struct device_node *dn, char
> > *name, int index)
> > {
> > if (!dn || dn != of_stdout || console_set_on_cmdline)
> > return false;
> > - return !add_preferred_console(name, index, NULL);
> > + return !add_preferred_console(name, index, of_stdout_options);
> > }
> > EXPORT_SYMBOL_GPL(of_console_check);
> >
> > --
> > 1.9.1
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
Hi Leif,
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
> Support specifying console options (like with console=ttyXN,)
> by appending them to the stdout-path property after a separating ':'.
>
> Example:
> stdout-path = "uart0:115200";
I would very much like to be able to use this -- it will allow
distributions to boot on a board without having to know _anything_ about
the console UART until userspace is up, which would make it possible to
have a completely generic installer image, without requiring the
platform to provide bootargs.
My only concern is that this conflates the set of kernel command line
options for the UART wit the DT binding for it. I don't know how good
people are at keeping those options stable, and I know they are
currently not documented -- we would need to add a stdout-path options
section to relevant UART bindings.
Thanks,
Mark.
>
> This patch also modifies of_find_node_by_path() to match only the
> portion of the path before a ':'.
>
> Signed-off-by: Leif Lindholm
> ---
> drivers/of/base.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 3823edf..89c6b33 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
> struct device_node *of_chosen;
> struct device_node *of_aliases;
> struct device_node *of_stdout;
> +static char *of_stdout_options;
>
> struct kset *of_kset;
>
> @@ -703,6 +704,8 @@ static struct device_node *__of_find_node_by_path(struct
> device_node *parent,
> if (!len)
> return NULL;
>
> + len = strchrnul(path, ':') - path;
> +
> __for_each_child_of_node(parent, child) {
> const char *name = strrchr(child->full_name, '/');
> if (WARN(!name, "malformed device_node %s\n", child->full_name))
> @@ -747,6 +750,8 @@ struct device_node *of_find_node_by_path(const char *path)
> if (!of_aliases)
> return NULL;
>
> + len = strchrnul(path, ':') - path;
> +
> for_each_property_of_node(of_aliases, pp) {
> if (strlen(pp->name) == len && !strncmp(pp->name, path,
> len)) {
> np = of_find_node_by_path(pp->value);
> @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
> align))
> name = of_get_property(of_chosen, "linux,stdout-path",
> NULL);
> if (IS_ENABLED(CONFIG_PPC) && !name)
> name = of_get_property(of_aliases, "stdout", NULL);
> - if (name)
> + if (name) {
> of_stdout = of_find_node_by_path(name);
> + of_stdout_options = strchr(name, ':');
> + if (of_stdout_options != NULL)
> + of_stdout_options++;
> + }
> }
>
> if (!of_aliases)
> @@ -1957,7 +1966,7 @@ bool of_console_check(struct device_node *dn, char
> *name, int index)
> {
> if (!dn || dn != of_stdout || console_set_on_cmdline)
> return false;
> - return !add_preferred_console(name, index, NULL);
> + return !add_preferred_console(name, index, of_stdout_options);
> }
> EXPORT_SYMBOL_GPL(of_console_check);
>
> --
> 1.9.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
Hi Leif,
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
Support specifying console options (like with console=ttyXN,options)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = uart0:115200;
I would very much like to be able to use this -- it will allow
distributions to boot on a board without having to know _anything_ about
the console UART until userspace is up, which would make it possible to
have a completely generic installer image, without requiring the
platform to provide bootargs.
My only concern is that this conflates the set of kernel command line
options for the UART wit the DT binding for it. I don't know how good
people are at keeping those options stable, and I know they are
currently not documented -- we would need to add a stdout-path options
section to relevant UART bindings.
Thanks,
Mark.
This patch also modifies of_find_node_by_path() to match only the
portion of the path before a ':'.
Signed-off-by: Leif Lindholm leif.lindh...@linaro.org
---
drivers/of/base.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3823edf..89c6b33 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
struct device_node *of_chosen;
struct device_node *of_aliases;
struct device_node *of_stdout;
+static char *of_stdout_options;
struct kset *of_kset;
@@ -703,6 +704,8 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
if (!len)
return NULL;
+ len = strchrnul(path, ':') - path;
+
__for_each_child_of_node(parent, child) {
const char *name = strrchr(child-full_name, '/');
if (WARN(!name, malformed device_node %s\n, child-full_name))
@@ -747,6 +750,8 @@ struct device_node *of_find_node_by_path(const char *path)
if (!of_aliases)
return NULL;
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name, path,
len)) {
np = of_find_node_by_path(pp-value);
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, linux,stdout-path,
NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
}
if (!of_aliases)
@@ -1957,7 +1966,7 @@ bool of_console_check(struct device_node *dn, char
*name, int index)
{
if (!dn || dn != of_stdout || console_set_on_cmdline)
return false;
- return !add_preferred_console(name, index, NULL);
+ return !add_preferred_console(name, index, of_stdout_options);
}
EXPORT_SYMBOL_GPL(of_console_check);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote:
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
Support specifying console options (like with console=ttyXN,options)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = uart0:115200;
I would very much like to be able to use this -- it will allow
distributions to boot on a board without having to know _anything_ about
the console UART until userspace is up, which would make it possible to
have a completely generic installer image, without requiring the
platform to provide bootargs.
My only concern is that this conflates the set of kernel command line
options for the UART wit the DT binding for it. I don't know how good
people are at keeping those options stable, and I know they are
currently not documented -- we would need to add a stdout-path options
section to relevant UART bindings.
I don't disagree.
Current options are fairly well defined and stable, at least for any
driver that uses uart_parse_options() (documented in
Documentation/serial/driver).
From drivers/tty/serial/serial_core.c:
* uart_parse_options decodes a string containing the serial console
* options. The format of the string is baudparitybitsflow,
* eg: 115200n8r
/
Leif
This patch also modifies of_find_node_by_path() to match only the
portion of the path before a ':'.
Signed-off-by: Leif Lindholm leif.lindh...@linaro.org
---
drivers/of/base.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3823edf..89c6b33 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
struct device_node *of_chosen;
struct device_node *of_aliases;
struct device_node *of_stdout;
+static char *of_stdout_options;
struct kset *of_kset;
@@ -703,6 +704,8 @@ static struct device_node
*__of_find_node_by_path(struct device_node *parent,
if (!len)
return NULL;
+ len = strchrnul(path, ':') - path;
+
__for_each_child_of_node(parent, child) {
const char *name = strrchr(child-full_name, '/');
if (WARN(!name, malformed device_node %s\n, child-full_name))
@@ -747,6 +750,8 @@ struct device_node *of_find_node_by_path(const char
*path)
if (!of_aliases)
return NULL;
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name, path,
len)) {
np = of_find_node_by_path(pp-value);
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, linux,stdout-path,
NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
}
if (!of_aliases)
@@ -1957,7 +1966,7 @@ bool of_console_check(struct device_node *dn, char
*name, int index)
{
if (!dn || dn != of_stdout || console_set_on_cmdline)
return false;
- return !add_preferred_console(name, index, NULL);
+ return !add_preferred_console(name, index, of_stdout_options);
}
EXPORT_SYMBOL_GPL(of_console_check);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 2014-11-25 at 11:17 +, Leif Lindholm wrote:
On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote:
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
Support specifying console options (like with console=ttyXN,options)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = uart0:115200;
I would very much like to be able to use this -- it will allow
distributions to boot on a board without having to know _anything_ about
the console UART until userspace is up, which would make it possible to
have a completely generic installer image, without requiring the
platform to provide bootargs.
My only concern is that this conflates the set of kernel command line
options for the UART wit the DT binding for it. I don't know how good
people are at keeping those options stable, and I know they are
currently not documented -- we would need to add a stdout-path options
section to relevant UART bindings.
I don't disagree.
Current options are fairly well defined and stable, at least for any
driver that uses uart_parse_options() (documented in
Documentation/serial/driver).
My concern is that this is Linux specific, other OSes may have different
ideas about how stdout options should be formatted within this property.
(At least I don't know of any standardisation of the 115200n8 thing --
I'd love to be corrected!)
If I were a firmware author I'd be wary of specifying a stdout-path with
a Linux specific suffix.
Search ePAPR for baud it seems that the generic serial binding includes
a current-speed property in 6.2.1.2. It then goes on a bit ambiguously
to talk about the NS16550 in 6.2.2 but I think 6.2.1.2 was intended to
be generic. No mention of stop-bits/parity etc though, they are assumed
to be set already I think
One thought I had was to define a dt-stdout pseudo-console so that
console=dt-stdout,115200n8 or something could be used.
Anyway I applied your patch to v3.18-rc5 and ran it on a Mustang and it
didn't work for some reason. I'm using:
fdt set /chosen stdout-path /soc/serial@1c02:115200
setenv bootargs earlycon=uart8250,mmio32,0x1c02 root=/dev/sda3 rw
debug
So I get earlycon but then no proper console. Removing earlycon just
makes that stop working.
With:
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 89c6b33..5dc1718 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1840,6 +1840,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
of_stdout_options = strchr(name, ':');
if (of_stdout_options != NULL)
of_stdout_options++;
+ printk(KERN_CRIT %s: name=%s of_stdout=%p
options=%s\n,
+ __func__, name, of_stdout, of_stdout_options);
}
}
I can see in dmesg:
[0.00] of_alias_scan: name=/soc/serial@1c02:115200 of_stdout=
(null) options=115200
So it seems like of_find_node_by_path() is confused by the :.
I've not tried it but I'd have expected something more like:
if (name) {
of_stdout_options = strchr(name, ':');
if (of_stdout_options != NULL) {
*of_stdout_options = '0';
of_stdout_options++;
}
of_stdout = of_find_node_by_path(name);
}
i.e. strip the options then do the patch lookup. name is const so this
won't actually work, but something like it...
Ian.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote: Hi Leif, On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: Support specifying console options (like with console=ttyXN,options) by appending them to the stdout-path property after a separating ':'. Example: stdout-path = uart0:115200; I would very much like to be able to use this -- it will allow distributions to boot on a board without having to know _anything_ about the console UART until userspace is up, which would make it possible to have a completely generic installer image, without requiring the platform to provide bootargs. Note there are several serial drivers that read back the console options from the hardware assuming it has been initialized correctly by the bootloader already. So when your bootloader already made some output on the console (which is likely) you can do that already. Of course this doesn't solve all problems, like if you want to use a different console for the kernel than for the bootloader. Anyway, this behaviour is very nice since the kernel always uses the same speed for the console as the bootloader does. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 12:07:29PM +, Ian Campbell wrote:
My concern is that this is Linux specific, other OSes may have different
ideas about how stdout options should be formatted within this property.
(At least I don't know of any standardisation of the 115200n8 thing --
I'd love to be corrected!)
If I were a firmware author I'd be wary of specifying a stdout-path with
a Linux specific suffix.
Search ePAPR for baud it seems that the generic serial binding includes
a current-speed property in 6.2.1.2. It then goes on a bit ambiguously
to talk about the NS16550 in 6.2.2 but I think 6.2.1.2 was intended to
be generic. No mention of stop-bits/parity etc though, they are assumed
to be set already I think
One thought I had was to define a dt-stdout pseudo-console so that
console=dt-stdout,115200n8 or something could be used.
I'll wait for others to comment on the above.
Anyway I applied your patch to v3.18-rc5 and ran it on a Mustang and it
didn't work for some reason. I'm using:
fdt set /chosen stdout-path /soc/serial@1c02:115200
setenv bootargs earlycon=uart8250,mmio32,0x1c02 root=/dev/sda3
rw debug
So I get earlycon but then no proper console. Removing earlycon just
makes that stop working.
Right, so having debugged this a bit offline, I'm amazed I booted
anything at all, given how badly I broke path scanning...
Please ignore previous version - a fixed one follows:
/
Leif
From aef87fd958902afe881720286d525e10997462b8 Mon Sep 17 00:00:00 2001
From: Leif Lindholm leif.lindh...@linaro.org
Date: Mon, 24 Nov 2014 22:23:58 +
Subject: [PATCH] of: support passing console options with stdout-path
Support specifying console options (like with console=ttyXN,options)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = uart0:115200;
This patch also modifies of_find_node_by_path() to match only the
portion of the path before a ':'.
Signed-off-by: Leif Lindholm leif.lindh...@linaro.org
---
drivers/of/base.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3823edf..ecd6290 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
struct device_node *of_chosen;
struct device_node *of_aliases;
struct device_node *of_stdout;
+static char *of_stdout_options;
struct kset *of_kset;
@@ -699,10 +700,15 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
{
struct device_node *child;
int len = strchrnul(path, '/') - path;
+ int term;
if (!len)
return NULL;
+ term = strchrnul(path, ':') - path;
+ if (term len)
+ len = term;
+
__for_each_child_of_node(parent, child) {
const char *name = strrchr(child-full_name, '/');
if (WARN(!name, malformed device_node %s\n, child-full_name))
@@ -742,11 +748,16 @@ struct device_node *of_find_node_by_path(const char *path)
if (*path != '/') {
char *p = strchrnul(path, '/');
int len = p - path;
+ int term;
/* of_aliases must not be NULL */
if (!of_aliases)
return NULL;
+ term = strchrnul(path, ':') - path;
+ if (term len)
+ len = term;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name, path,
len)) {
np = of_find_node_by_path(pp-value);
@@ -1830,8 +1841,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, linux,stdout-path,
NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
}
if (!of_aliases)
@@ -1957,7 +1972,7 @@ bool of_console_check(struct device_node *dn, char *name,
int index)
{
if (!dn || dn != of_stdout || console_set_on_cmdline)
return false;
- return !add_preferred_console(name, index, NULL);
+ return !add_preferred_console(name, index, of_stdout_options);
}
EXPORT_SYMBOL_GPL(of_console_check);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 25 Nov 2014 00:00:16 +0100 , Andrew Lunn and...@lunn.ch wrote: On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: Support specifying console options (like with console=ttyXN,options) by appending them to the stdout-path property after a separating ':'. Example: stdout-path = uart0:115200; This patch also modifies of_find_node_by_path() to match only the portion of the path before a ':'. Hi Leif These appears to somewhat conform to ePAPR, which says: A string that specifies the full path to the node representing the device to be used for boot console output. If the character : is present in the value it terminates the path. So you can put any random junk after the :. However, are we going to have backward/forward compatibility problems, and problems with bootloaders? The current kernel code does not look for the :. So a new DT blob on an old kernel will not work so well. For DT, we've generally not been worrying about compatibility in that direction. If we've got a new firmware, we should be able to get a new kernel. Nobody has really screamed bout it yet. More worrying, barebox does not support the : either. So there is a danger your bootloader suddenly goes silent after a dt blob update. Is barebox parsing the property? Or updating it? Or both? I've got no problem with this patch, but we can also support things like the 'current-speed' property in the UART to set a default when the stdout path doesn't have any arguments. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Mon, 24 Nov 2014 22:23:58 +
, Leif Lindholm leif.lindh...@linaro.org
wrote:
Support specifying console options (like with console=ttyXN,options)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = uart0:115200;
This patch also modifies of_find_node_by_path() to match only the
portion of the path before a ':'.
Signed-off-by: Leif Lindholm leif.lindh...@linaro.org
---
drivers/of/base.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3823edf..89c6b33 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -37,6 +37,7 @@ EXPORT_SYMBOL(of_allnodes);
struct device_node *of_chosen;
struct device_node *of_aliases;
struct device_node *of_stdout;
+static char *of_stdout_options;
struct kset *of_kset;
@@ -703,6 +704,8 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
if (!len)
return NULL;
+ len = strchrnul(path, ':') - path;
+
__for_each_child_of_node(parent, child) {
const char *name = strrchr(child-full_name, '/');
if (WARN(!name, malformed device_node %s\n, child-full_name))
@@ -747,6 +750,8 @@ struct device_node *of_find_node_by_path(const char *path)
if (!of_aliases)
return NULL;
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name, path,
len)) {
np = of_find_node_by_path(pp-value);
Can you add a testcase to drivers/of/unittests.c for this new path
parsing? Then we know it's correct!
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, linux,stdout-path,
NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
Not so good. The ':' should actually be a generic part of
of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
There are other places that would use it. I would add a second,
optional, argument to of_find_node_by_path() that will update a pointer
to the arguments after the ':'.
g.
}
if (!of_aliases)
@@ -1957,7 +1966,7 @@ bool of_console_check(struct device_node *dn, char
*name, int index)
{
if (!dn || dn != of_stdout || console_set_on_cmdline)
return false;
- return !add_preferred_console(name, index, NULL);
+ return !add_preferred_console(name, index, of_stdout_options);
}
EXPORT_SYMBOL_GPL(of_console_check);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 25 Nov 2014 12:07:29 +
, Ian Campbell i...@debian.org
wrote:
On Tue, 2014-11-25 at 11:17 +, Leif Lindholm wrote:
On Tue, Nov 25, 2014 at 10:35:04AM +, Mark Rutland wrote:
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote:
Support specifying console options (like with console=ttyXN,options)
by appending them to the stdout-path property after a separating ':'.
Example:
stdout-path = uart0:115200;
I would very much like to be able to use this -- it will allow
distributions to boot on a board without having to know _anything_ about
the console UART until userspace is up, which would make it possible to
have a completely generic installer image, without requiring the
platform to provide bootargs.
My only concern is that this conflates the set of kernel command line
options for the UART wit the DT binding for it. I don't know how good
people are at keeping those options stable, and I know they are
currently not documented -- we would need to add a stdout-path options
section to relevant UART bindings.
I don't disagree.
Current options are fairly well defined and stable, at least for any
driver that uses uart_parse_options() (documented in
Documentation/serial/driver).
My concern is that this is Linux specific, other OSes may have different
ideas about how stdout options should be formatted within this property.
(At least I don't know of any standardisation of the 115200n8 thing --
I'd love to be corrected!)
If I were a firmware author I'd be wary of specifying a stdout-path with
a Linux specific suffix.
Search ePAPR for baud it seems that the generic serial binding includes
a current-speed property in 6.2.1.2. It then goes on a bit ambiguously
to talk about the NS16550 in 6.2.2 but I think 6.2.1.2 was intended to
be generic. No mention of stop-bits/parity etc though, they are assumed
to be set already I think
One thought I had was to define a dt-stdout pseudo-console so that
console=dt-stdout,115200n8 or something could be used.
Anyway I applied your patch to v3.18-rc5 and ran it on a Mustang and it
didn't work for some reason. I'm using:
fdt set /chosen stdout-path /soc/serial@1c02:115200
setenv bootargs earlycon=uart8250,mmio32,0x1c02 root=/dev/sda3
rw debug
So I get earlycon but then no proper console. Removing earlycon just
makes that stop working.
With:
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 89c6b33..5dc1718 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1840,6 +1840,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
of_stdout_options = strchr(name, ':');
if (of_stdout_options != NULL)
of_stdout_options++;
+ printk(KERN_CRIT %s: name=%s of_stdout=%p
options=%s\n,
+__func__, name, of_stdout, of_stdout_options);
}
}
I can see in dmesg:
[0.00] of_alias_scan: name=/soc/serial@1c02:115200 of_stdout=
(null) options=115200
So it seems like of_find_node_by_path() is confused by the :.
That would be a bug in of_find_node_by_path(). Should add a test case
for that because ':' is supposed to terminate path parsing.
I've not tried it but I'd have expected something more like:
if (name) {
of_stdout_options = strchr(name, ':');
if (of_stdout_options != NULL) {
*of_stdout_options = '0';
of_stdout_options++;
}
of_stdout = of_find_node_by_path(name);
}
This isn't really right because it modifies the property data which is
supposed to be const. of_find_node_by_path() already does the search
piecewise, so it should be fairly straightforward to add a test for the
':'. Anyone care to take a look? If I need to do it then it will take a couple
of weeks before I've got some free time. :-/
i.e. strip the options then do the patch lookup. name is const so this
won't actually work, but something like it...
Ian.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name, path,
len)) {
np = of_find_node_by_path(pp-value);
Can you add a testcase to drivers/of/unittests.c for this new path
parsing? Then we know it's correct!
Will do.
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, linux,stdout-path,
NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
Not so good. The ':' should actually be a generic part of
of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
There are other places that would use it. I would add a second,
optional, argument to of_find_node_by_path() that will update a pointer
to the arguments after the ':'.
So, I agree this would be nicer...
However, I'm counting 157 callers of this function outside of
drivers/of and 43 inside. Any chance you'd let me get away with a
separate of_find_extra_options_by_path()?
/
Leif
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm leif.lindh...@linaro.org wrote:
On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name,
path, len)) {
np = of_find_node_by_path(pp-value);
Can you add a testcase to drivers/of/unittests.c for this new path
parsing? Then we know it's correct!
Will do.
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align))
name = of_get_property(of_chosen, linux,stdout-path,
NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
Not so good. The ':' should actually be a generic part of
of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
There are other places that would use it. I would add a second,
optional, argument to of_find_node_by_path() that will update a pointer
to the arguments after the ':'.
So, I agree this would be nicer...
However, I'm counting 157 callers of this function outside of
drivers/of and 43 inside. Any chance you'd let me get away with a
separate of_find_extra_options_by_path()?
I'd rather not. It is a trivial change and can be pretty much done
mechanically. Something like (I think; I always have to look up the
sed syntax):
sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
If it turns out to be really painful, then I'll relent, but I'd like
you to at least try! :-)
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 2014-11-25 at 15:20 +, Grant Likely wrote:
On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm leif.lindh...@linaro.org
wrote:
On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name,
path, len)) {
np = of_find_node_by_path(pp-value);
Can you add a testcase to drivers/of/unittests.c for this new path
parsing? Then we know it's correct!
Will do.
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size,
u64 align))
name = of_get_property(of_chosen,
linux,stdout-path, NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
Not so good. The ':' should actually be a generic part of
of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
There are other places that would use it. I would add a second,
optional, argument to of_find_node_by_path() that will update a pointer
to the arguments after the ':'.
So, I agree this would be nicer...
However, I'm counting 157 callers of this function outside of
drivers/of and 43 inside. Any chance you'd let me get away with a
separate of_find_extra_options_by_path()?
I'd rather not. It is a trivial change and can be pretty much done
mechanically. Something like (I think; I always have to look up the
sed syntax):
sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
Coccinelle rules for this sort of transformation...
Ian.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 3:24 PM, Ian Campbell i...@debian.org wrote:
On Tue, 2014-11-25 at 15:20 +, Grant Likely wrote:
On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm leif.lindh...@linaro.org
wrote:
On Tue, Nov 25, 2014 at 02:58:54PM +, Grant Likely wrote:
+ len = strchrnul(path, ':') - path;
+
for_each_property_of_node(of_aliases, pp) {
if (strlen(pp-name) == len !strncmp(pp-name,
path, len)) {
np = of_find_node_by_path(pp-value);
Can you add a testcase to drivers/of/unittests.c for this new path
parsing? Then we know it's correct!
Will do.
@@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size,
u64 align))
name = of_get_property(of_chosen,
linux,stdout-path, NULL);
if (IS_ENABLED(CONFIG_PPC) !name)
name = of_get_property(of_aliases, stdout, NULL);
- if (name)
+ if (name) {
of_stdout = of_find_node_by_path(name);
+ of_stdout_options = strchr(name, ':');
+ if (of_stdout_options != NULL)
+ of_stdout_options++;
+ }
Not so good. The ':' should actually be a generic part of
of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
There are other places that would use it. I would add a second,
optional, argument to of_find_node_by_path() that will update a pointer
to the arguments after the ':'.
So, I agree this would be nicer...
However, I'm counting 157 callers of this function outside of
drivers/of and 43 inside. Any chance you'd let me get away with a
separate of_find_extra_options_by_path()?
I'd rather not. It is a trivial change and can be pretty much done
mechanically. Something like (I think; I always have to look up the
sed syntax):
sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
Coccinelle rules for this sort of transformation...
/me still hasn't gotten his head around how to use Coccinelle properly. :-(
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, 2014-11-25 at 15:39 +, Grant Likely wrote: On Tue, Nov 25, 2014 at 3:24 PM, Ian Campbell i...@debian.org wrote: sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\) Coccinelle rules for this sort of transformation... /me still hasn't gotten his head around how to use Coccinelle properly. :-( Me neither, but for the add an extra NULL parameter case I can generally find something to cargo cult from ;-) (it's been ages since I had a need though) Ian. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
More worrying, barebox does not support the : either. So there is a danger your bootloader suddenly goes silent after a dt blob update. Is barebox parsing the property? Or updating it? Or both? I've got no problem with this patch, but we can also support things like the 'current-speed' property in the UART to set a default when the stdout path doesn't have any arguments. Hi Grant I'm no export with barebox. So i just grep'ed the code. It parses the property. Also as with linux, of_find_node_by_path() does not stop at : as you say it should. There is one case of the property being updated, but that is only for the PPC mpc85xx. Barebox does not appear to use 'current-speed' at all. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 12:00:16AM +0100, Andrew Lunn wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: > > Support specifying console options (like with console=ttyXN,) > > by appending them to the stdout-path property after a separating ':'. > > > > Example: > > stdout-path = "uart0:115200"; > > > > This patch also modifies of_find_node_by_path() to match only the > > portion of the path before a ':'. > > Hi Leif > > These appears to somewhat conform to ePAPR, which says: > > A string that specifies the full path to the node representing > the device to be used for boot console output. If the character > ":" is present in the value it terminates the path. > > So you can put any random junk after the :. However, are we going to > have backward/forward compatibility problems, and problems with > bootloaders? The current kernel code does not look for the :. So a new > DT blob on an old kernel will not work so well. > > More worrying, barebox does not support the : either. So there is a > danger your bootloader suddenly goes silent after a dt blob update. Since in barebox we use the upstream Kernel devicetrees it will be under our control when they arrive, so we can add support for that ':' before the new devicetrees arrive. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 12:00:16AM +0100, Andrew Lunn wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: > > Support specifying console options (like with console=ttyXN,) > > by appending them to the stdout-path property after a separating ':'. > > > > Example: > > stdout-path = "uart0:115200"; > > > > This patch also modifies of_find_node_by_path() to match only the > > portion of the path before a ':'. > > Hi Leif > > These appears to somewhat conform to ePAPR, which says: > > A string that specifies the full path to the node representing > the device to be used for boot console output. If the character > ":" is present in the value it terminates the path. > > So you can put any random junk after the :. However, are we going to > have backward/forward compatibility problems, and problems with > bootloaders? The current kernel code does not look for the :. So a new > DT blob on an old kernel will not work so well. I _think_ this will be less of a problem in practice than it could be in theory. The reason this is needed is that at least several platforms have different baudrate settings in firmware than the default provided by the kernel for their UART. As a result, stdout-path becomes semi-useless; the only thing it gives you is the ability to get earlycon output without specifying a specific device (and then the console turns to garbage once non-earlycon is enabled). Hence, a platform that gets along happily today without the ability to specify console options in stdout-path would have no pressing need to add it to its dt. This should permit at least a very long, soft, transition path. (console= on kernel command line continues to override stdout-path.) > More worrying, barebox does not support the : either. So there is a > danger your bootloader suddenly goes silent after a dt blob update. _That_ would be most unfortunate. > Would it be safer to add a new property in chosen? My preference would be not, given the above, but the important thing is to get the functionality in so we get rid of mandatory console=. / Leif -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: > Support specifying console options (like with console=ttyXN,) > by appending them to the stdout-path property after a separating ':'. > > Example: > stdout-path = "uart0:115200"; > > This patch also modifies of_find_node_by_path() to match only the > portion of the path before a ':'. Hi Leif These appears to somewhat conform to ePAPR, which says: A string that specifies the full path to the node representing the device to be used for boot console output. If the character ":" is present in the value it terminates the path. So you can put any random junk after the :. However, are we going to have backward/forward compatibility problems, and problems with bootloaders? The current kernel code does not look for the :. So a new DT blob on an old kernel will not work so well. More worrying, barebox does not support the : either. So there is a danger your bootloader suddenly goes silent after a dt blob update. Would it be safer to add a new property in chosen? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: Support specifying console options (like with console=ttyXN,options) by appending them to the stdout-path property after a separating ':'. Example: stdout-path = uart0:115200; This patch also modifies of_find_node_by_path() to match only the portion of the path before a ':'. Hi Leif These appears to somewhat conform to ePAPR, which says: A string that specifies the full path to the node representing the device to be used for boot console output. If the character : is present in the value it terminates the path. So you can put any random junk after the :. However, are we going to have backward/forward compatibility problems, and problems with bootloaders? The current kernel code does not look for the :. So a new DT blob on an old kernel will not work so well. More worrying, barebox does not support the : either. So there is a danger your bootloader suddenly goes silent after a dt blob update. Would it be safer to add a new property in chosen? Andrew -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 12:00:16AM +0100, Andrew Lunn wrote: On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: Support specifying console options (like with console=ttyXN,options) by appending them to the stdout-path property after a separating ':'. Example: stdout-path = uart0:115200; This patch also modifies of_find_node_by_path() to match only the portion of the path before a ':'. Hi Leif These appears to somewhat conform to ePAPR, which says: A string that specifies the full path to the node representing the device to be used for boot console output. If the character : is present in the value it terminates the path. So you can put any random junk after the :. However, are we going to have backward/forward compatibility problems, and problems with bootloaders? The current kernel code does not look for the :. So a new DT blob on an old kernel will not work so well. I _think_ this will be less of a problem in practice than it could be in theory. The reason this is needed is that at least several platforms have different baudrate settings in firmware than the default provided by the kernel for their UART. As a result, stdout-path becomes semi-useless; the only thing it gives you is the ability to get earlycon output without specifying a specific device (and then the console turns to garbage once non-earlycon is enabled). Hence, a platform that gets along happily today without the ability to specify console options in stdout-path would have no pressing need to add it to its dt. This should permit at least a very long, soft, transition path. (console= on kernel command line continues to override stdout-path.) More worrying, barebox does not support the : either. So there is a danger your bootloader suddenly goes silent after a dt blob update. _That_ would be most unfortunate. Would it be safer to add a new property in chosen? My preference would be not, given the above, but the important thing is to get the functionality in so we get rid of mandatory console=. / Leif -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: support passing console options with stdout-path
On Tue, Nov 25, 2014 at 12:00:16AM +0100, Andrew Lunn wrote: On Mon, Nov 24, 2014 at 10:23:58PM +, Leif Lindholm wrote: Support specifying console options (like with console=ttyXN,options) by appending them to the stdout-path property after a separating ':'. Example: stdout-path = uart0:115200; This patch also modifies of_find_node_by_path() to match only the portion of the path before a ':'. Hi Leif These appears to somewhat conform to ePAPR, which says: A string that specifies the full path to the node representing the device to be used for boot console output. If the character : is present in the value it terminates the path. So you can put any random junk after the :. However, are we going to have backward/forward compatibility problems, and problems with bootloaders? The current kernel code does not look for the :. So a new DT blob on an old kernel will not work so well. More worrying, barebox does not support the : either. So there is a danger your bootloader suddenly goes silent after a dt blob update. Since in barebox we use the upstream Kernel devicetrees it will be under our control when they arrive, so we can add support for that ':' before the new devicetrees arrive. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
