"of: fix handling of '/' in options for of_find_node_by_path()" breaks stdout-path

While updating my local working tree to 4.0-rc4 this morning I noticed that I no longer got any console (neither video output not serial console) on an Allwinner A20 ARM board.

This is caused by:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/of?id=106937e8ccdcf0f4b95fbf0fe9abd42766cade33

Reverting this commit fixes the serial console being gone for me.

After this there still is an issue that tty0 no longer is seen as console, where it used to be properly used as console in 3.19, I'll investigate that further and send a separate mail about this.

Greg, this commit has a: "Cc: <[email protected]> # 3.19" please do not apply this commit to stable!

u-boot sets stdout-path to this value on the board in question:
"/soc@01c00000/serial@01c28000:115200"

Looking at the first hunk of the patch in question, the problem is obvious:

(quoted diff) +7 -6

@@ -714,16 +714,17 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
 const char *path)
 {
  struct device_node *child;
- int len = strchrnul(path, '/') - path;
- int term;
+ int len;
+ const char *end;
+ end = strchr(path, ':');
+ if (!end)
+ end = strchrnul(path, '/');
+
+ len = end - path;
  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))
 

The new code to determine len will match (when starting at root) the name of all child nodes against: "soc@01c00000/serial@01c28000" as it checks for the ":" first and then uses everything before it. Where as the old code would match against: "soc@01c00000" which is the correct thing to do.

The best fix I can come up with is to check for both ":" and "/" and use the earlier one as end to calculate the length. I've not coded this out / tested this due to -ENOTIME. Note that I've also not audited the rest of the patch for similar issues.

I will happily test any patches to fix this.

From cbb150ddd277e5fe1c109e6a675f075f0611f71d Mon Sep 17 00:00:00 2001

Hi Hans,

Quoted 8 lines · Hans de Goede

While updating my local working tree to 4.0-rc4 this morning I noticed that I no longer got any console (neither video output not serial console) on an Allwinner A20 ARM board.

This is caused by:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/of?id=106937e8ccdcf0f4b95fbf0fe9abd42766cade33

Reverting this commit fixes the serial console being gone for me.

Sorry about that.

Quoted 40 lines

After this there still is an issue that tty0 no longer is seen as console, where it used to be properly used as console in 3.19, I'll investigate that further and send a separate mail about this.

Greg, this commit has a: "Cc: <[email protected]> # 3.19" please do not apply this commit to stable!

u-boot sets stdout-path to this value on the board in question:
"/soc@01c00000/serial@01c28000:115200"

Looking at the first hunk of the patch in question, the problem is obvious:

(quoted diff) +7 -6

@@ -714,16 +714,17 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
  const char *path)
  {
   struct device_node *child;
- int len = strchrnul(path, '/') - path;
- int term;
+ int len;
+ const char *end;
+ end = strchr(path, ':');
+ if (!end)
+ end = strchrnul(path, '/');
+
+ len = end - path;
   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))
 

The new code to determine len will match (when starting at root) the name of all child nodes against: "soc@01c00000/serial@01c28000" as it checks for the ":" first and then uses everything before it. Where as the old code would match against: "soc@01c00000" which is the correct thing to do.

Quoted 6 lines

The best fix I can come up with is to check for both ":" and "/" and use the earlier one as end to calculate the length. I've not coded this out / tested this due to -ENOTIME. Note that I've also not audited the rest of the patch for similar issues.

I will happily test any patches to fix this.

Can you give this one a try for the first part of your problem?

And if you're happy with that, I can revert the previous version and send a new combined fix.

Rob/Grant: am I OK to assume the existence of the phandle-tests subnode for my unrelated test?

/
Leif

From cbb150ddd277e5fe1c109e6a675f075f0611f71d Mon Sep 17 00:00:00 2001
From: Leif Lindholm <[email protected]>
Date: Mon, 16 Mar 2015 17:58:22 +0000
Subject: [PATCH] of: fix regression in of_find_node_opts_by_path()

This fixes a regression for dealing with paths that contain both a ':' option separator and a '/' in the path preceding it. And adds a test case to prove it.

Fixes 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")

Reported-by Hans de Goede <[email protected]>

Signed-off-by Leif Lindholm <[email protected]>

---
  drivers/of/base.c     | 10 ++++------
  drivers/of/unittest.c |  5 +++++
  2 files changed, 9 insertions(+), 6 deletions(-)

drivers/of/base.c +4 -6

index adb8764..2ee7265 100644
@@ -715,13 +715,11 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
  {
  	struct device_node *child;
  	int len;
-	const char *end;
+	const char *p1, *p2;
 
-	end = strchr(path, ':');
-	if (!end)
-		end = strchrnul(path, '/');
-
-	len = end - path;
+	p1 = strchrnul(path, ':');
+	p2 = strchrnul(path, '/');
+	len = (p1 < p2 ? p1 : p2) - path;
  	if (!len)
  		return NULL;
 

drivers/of/unittest.c +0 -0

index aba8946..8d94349 100644
@@ -97,6 +97,11 @@ static void __init of_selftest_find_node_by_name(void)
  		 "option path test, subcase #1 failed\n");