| Message ID | 1416867838-18652-1-git-send-email-leif.lindholm@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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
On Tue, Nov 25, 2014 at 12:00:16AM +0100, Andrew Lunn wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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
On Tue, Nov 25, 2014 at 12:00:16AM +0100, Andrew Lunn wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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
Hi Leif, On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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.lindholm@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 > >
On Tue, Nov 25, 2014 at 10:35:04AM +0000, Mark Rutland wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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 <baud><parity><bits><flow>, * 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.lindholm@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 > > > >
On Tue, Nov 25, 2014 at 10:35:04AM +0000, Mark Rutland wrote: > Hi Leif, > > On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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
On Tue, 25 Nov 2014 00:00:16 +0100 , Andrew Lunn <andrew@lunn.ch> wrote: > On Mon, Nov 24, 2014 at 10:23:58PM +0000, 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.
On Mon, 24 Nov 2014 22:23:58 +0000 , Leif Lindholm <leif.lindholm@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.lindholm@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 >
On Tue, Nov 25, 2014 at 02:58:54PM +0000, 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
On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Nov 25, 2014 at 02:58:54PM +0000, 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.
On Tue, 2014-11-25 at 15:20 +0000, Grant Likely wrote: > On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Tue, Nov 25, 2014 at 02:58:54PM +0000, 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.
On Tue, Nov 25, 2014 at 3:24 PM, Ian Campbell <ijc@debian.org> wrote: > On Tue, 2014-11-25 at 15:20 +0000, Grant Likely wrote: >> On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > On Tue, Nov 25, 2014 at 02:58:54PM +0000, 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.
On Tue, 2014-11-25 at 15:39 +0000, Grant Likely wrote: > On Tue, Nov 25, 2014 at 3:24 PM, Ian Campbell <ijc@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.
> > 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
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);
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.lindholm@linaro.org> --- drivers/of/base.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)