[U-Boot] [PATCH] dm: serial: Handle "stdout-path" with ":options" correctly

[U-Boot] [PATCH] dm: serial: Handle "stdout-path" with ":options" correctly

[Bin Meng]

With commit f0921f5098d8 ("fdt: Sync up to the latest libfdt"),
SiFive Unleashed board does not boot any more. This was due to
the U-Boot local changes commit 77d7fff8cec2 ("fdt: Fix handling
of paths with options in them") to libfdt/fdt_ro.c was dropped
during the libfdt upgrade.

From the history [1] it was mentioned that the U-Boot changes
commit 77d7fff8cec2 ("fdt: Fix handling of paths with options in
them") was rejected by libfdt upstream, hence we need find another
way to fix the things.

This commit uses another method, by updating serial_check_stdout()
directly to handle the situation of "stdout-path" with ":options".
A simpler way is to change the logic in fdtdec_get_chosen_node()
to do similar thing, but I feel that not every property in chosen
node may have the option in them, hence it would make more sense
to do the special handling in serial_check_stdout() directly.

[1]: http://patchwork.ozlabs.org/patch/462756/

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

 drivers/serial/serial-uclass.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index dcdaede..391f4a1 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -30,28 +30,32 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
 static int serial_check_stdout(const void *blob, struct udevice **devp)
 {
 	int node;
+	const char *str, *p, *name;
+	int namelen;
 
 	/* Check for a chosen console */
-	node = fdtdec_get_chosen_node(blob, "stdout-path");
-	if (node < 0) {
-		const char *str, *p, *name;
-
-		/*
-		 * Deal with things like
-		 *	stdout-path = "serial0:115200n8";
-		 *
-		 * We need to look up the alias and then follow it to the
-		 * correct node.
-		 */
-		str = fdtdec_get_chosen_prop(blob, "stdout-path");
-		if (str) {
-			p = strchr(str, ':');
-			name = fdt_get_alias_namelen(blob, str,
-					p ? p - str : strlen(str));
+	str = fdtdec_get_chosen_prop(blob, "stdout-path");
+	if (str) {
+		p = strchr(str, ':');
+		namelen = p ? p - str : strlen(str);
+		node = fdt_path_offset_namelen(blob, str, namelen);
+
+		if (node < 0) {
+			/*
+			 * Deal with things like
+			 *	stdout-path = "serial0:115200n8";
+			 *
+			 * We need to look up the alias and then follow it to
+			 * the correct node.
+			 */
+			name = fdt_get_alias_namelen(blob, str, namelen);
 			if (name)
 				node = fdt_path_offset(blob, name);
 		}
+	} else {
+		node = -1;
 	}
+
 	if (node < 0)
 		node = fdt_path_offset(blob, "console");
 	if (!uclass_get_device_by_of_offset(UCLASS_SERIAL, node, devp))
-- 
2.7.4

[U-Boot] [PATCH] dm: serial: Handle "stdout-path" with ":options" correctly

[Simon Glass]

Hi Bin,

On Mon, 18 Nov 2019 at 09:12, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> With commit f0921f5098d8 ("fdt: Sync up to the latest libfdt"),
> SiFive Unleashed board does not boot any more. This was due to
> the U-Boot local changes commit 77d7fff8cec2 ("fdt: Fix handling
> of paths with options in them") to libfdt/fdt_ro.c was dropped
> during the libfdt upgrade.
>
> From the history [1] it was mentioned that the U-Boot changes
> commit 77d7fff8cec2 ("fdt: Fix handling of paths with options in
> them") was rejected by libfdt upstream, hence we need find another
> way to fix the things.
>
> This commit uses another method, by updating serial_check_stdout()
> directly to handle the situation of "stdout-path" with ":options".
> A simpler way is to change the logic in fdtdec_get_chosen_node()
> to do similar thing, but I feel that not every property in chosen
> node may have the option in them, hence it would make more sense
> to do the special handling in serial_check_stdout() directly.
>
> [1]: http://patchwork.ozlabs.org/patch/462756/
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
>  drivers/serial/serial-uclass.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Would it be possible to export serial_check_stdout() so we can add
tests? Or perhaps we should have tests for all of
serial_find_console_or_panic() since it is getting more and more
complicated and is not documented well.

Regards,
Simon

[U-Boot] [PATCH] dm: serial: Handle "stdout-path" with ":options" correctly

[Bin Meng]

Hi Simon,

On Tue, Nov 19, 2019 at 12:24 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 18 Nov 2019 at 09:12, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > With commit f0921f5098d8 ("fdt: Sync up to the latest libfdt"),
> > SiFive Unleashed board does not boot any more. This was due to
> > the U-Boot local changes commit 77d7fff8cec2 ("fdt: Fix handling
> > of paths with options in them") to libfdt/fdt_ro.c was dropped
> > during the libfdt upgrade.
> >
> > From the history [1] it was mentioned that the U-Boot changes
> > commit 77d7fff8cec2 ("fdt: Fix handling of paths with options in
> > them") was rejected by libfdt upstream, hence we need find another
> > way to fix the things.
> >
> > This commit uses another method, by updating serial_check_stdout()
> > directly to handle the situation of "stdout-path" with ":options".
> > A simpler way is to change the logic in fdtdec_get_chosen_node()
> > to do similar thing, but I feel that not every property in chosen
> > node may have the option in them, hence it would make more sense
> > to do the special handling in serial_check_stdout() directly.
> >
> > [1]: http://patchwork.ozlabs.org/patch/462756/
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > ---
> >
> >  drivers/serial/serial-uclass.c | 36 ++++++++++++++++++++----------------
> >  1 file changed, 20 insertions(+), 16 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Would it be possible to export serial_check_stdout() so we can add
> tests? Or perhaps we should have tests for all of
> serial_find_console_or_panic() since it is getting more and more
> complicated and is not documented well.

Yes, I think so. I sent a v2 with an improvement for readability but
did not have time to include testes. I may have to leave that for the
future.

Regards,
Bin