soc/tegra: refactor soc_is_tegra()

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +1 -3

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-	struct device_node *root;
+	struct of_device_id *match = tegra_machine_match;
 
-	root = of_find_node_by_path("/");
-	if (!root)

Quoted 30 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-	struct device_node *root;
+	struct of_device_id *match = tegra_machine_match;
 
-	root = of_find_node_by_path("/");
-	if (!root)
-		return false;
+	while(match->compatible){
+		if(of_machine_is_compatible(match->compatible))
+			return true;
+		match++;
+	}
 

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 34

Quoted 3 lines

drivers/soc/tegra/common.c +1 -1

-	return of_match_node(tegra_machine_match, root) != NULL;
+	return false;
 }

Ugh ... sorry, I thought that of_machine_is_compatible() looped through the matches. OK, let's stick with your initial fix.

Quoted 32 lines · Jon Hunter

Quoted 30 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-   struct device_node *root;
+   struct of_device_id *match = tegra_machine_match;
 
-   root = of_find_node_by_path("/");
-   if (!root)
-           return false;
+   while(match->compatible){
+           if(of_machine_is_compatible(match->compatible))
+                   return true;
+           match++;
+   }
 

Quoted 6 lines

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 34

Quoted 3 lines

drivers/soc/tegra/common.c +1 -1

-   return of_match_node(tegra_machine_match, root) != NULL;
+   return false;
 }

Ugh ... sorry, I thought that of_machine_is_compatible() looped through the matches. OK, let's stick with your initial fix.

Actually I prefer this one. Even if it is slightly more verbose, I think it's much clearer what's actually going on. Also this hides all of the OF node reference counting in a core function, so it's worth the extra line, in my opinion.

Thierry

Quoted 46 lines · Thierry Reding

Quoted 32 lines · Jon Hunter

Quoted 30 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-   struct device_node *root;
+   struct of_device_id *match = tegra_machine_match;
 
-   root = of_find_node_by_path("/");
-   if (!root)
-           return false;
+   while(match->compatible){
+           if(of_machine_is_compatible(match->compatible))
+                   return true;
+           match++;
+   }
 

Quoted 6 lines

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 34

Quoted 3 lines

drivers/soc/tegra/common.c +1 -1

-   return of_match_node(tegra_machine_match, root) != NULL;
+   return false;
 }

Ugh ... sorry, I thought that of_machine_is_compatible() looped through the matches. OK, let's stick with your initial fix.

Actually I prefer this one. Even if it is slightly more verbose, I think it's much clearer what's actually going on. Also this hides all of the OF node reference counting in a core function, so it's worth the extra line, in my opinion.

Quoted 1 line

Thierry

Hi Jon：

I like both, how aboout you?

Quoted 48 lines · Frank Lee

Quoted 46 lines · Thierry Reding

Quoted 32 lines · Jon Hunter

Quoted 30 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-   struct device_node *root;
+   struct of_device_id *match = tegra_machine_match;
 
-   root = of_find_node_by_path("/");
-   if (!root)
-           return false;
+   while(match->compatible){
+           if(of_machine_is_compatible(match->compatible))
+                   return true;
+           match++;
+   }
 

Quoted 6 lines

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 34

Quoted 3 lines

drivers/soc/tegra/common.c +1 -1

-   return of_match_node(tegra_machine_match, root) != NULL;
+   return false;
 }

Ugh ... sorry, I thought that of_machine_is_compatible() looped through the matches. OK, let's stick with your initial fix.

Actually I prefer this one. Even if it is slightly more verbose, I think it's much clearer what's actually going on. Also this hides all of the OF node reference counting in a core function, so it's worth the extra line, in my opinion.

Quoted 4 lines

Quoted 1 line

Thierry

Hi Jon：

I like both, how aboout you?

Yes fine with me too. However, looks like there is some formatting that needs to be fixed up above. Please make sure you run checkpatch.pl on your patches. Otherwise ...

Acked-by Jon Hunter <[email protected]>

Thierry, pick up this version if you prefer.

Quoted 61 lines · Jon Hunter

Quoted 48 lines · Frank Lee

Quoted 46 lines · Thierry Reding

Quoted 32 lines · Jon Hunter

Quoted 30 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-   struct device_node *root;
+   struct of_device_id *match = tegra_machine_match;
 
-   root = of_find_node_by_path("/");
-   if (!root)
-           return false;
+   while(match->compatible){
+           if(of_machine_is_compatible(match->compatible))
+                   return true;
+           match++;
+   }
 

Quoted 6 lines

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 34

Quoted 3 lines

drivers/soc/tegra/common.c +1 -1

-   return of_match_node(tegra_machine_match, root) != NULL;
+   return false;
 }

Ugh ... sorry, I thought that of_machine_is_compatible() looped through the matches. OK, let's stick with your initial fix.

Actually I prefer this one. Even if it is slightly more verbose, I think it's much clearer what's actually going on. Also this hides all of the OF node reference counting in a core function, so it's worth the extra line, in my opinion.

Quoted 4 lines

Quoted 1 line

Thierry

Hi Jon：

I like both, how aboout you?

Yes fine with me too. However, looks like there is some formatting that needs to be fixed up above. Please make sure you run checkpatch.pl on your patches. Otherwise ...

Acked-by Jon Hunter <[email protected]>

Quoted 1 line

Thierry, pick up this version if you prefer.

Yeah, I noticed the formatting issues, but I can take care of them while applying.

Thanks,
Thierry

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---
 drivers/soc/tegra/common.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/soc/tegra/common.c +7 -6

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-       struct device_node *root;
+       struct of_device_id *match = tegra_machine_match;
 
-       root = of_find_node_by_path("/");
-       if (!root)
-               return false;
+       while(match->compatible){
+               if(of_machine_is_compatible(match->compatible))
+                       return true;
+               match++;
+       }
 
-       return of_match_node(tegra_machine_match, root) != NULL;
+       return false;
 }
--

2.17.0

Quoted 8 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---

Quoted 20 lines

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-       struct device_node *root;
+       struct of_device_id *match = tegra_machine_match;
 
-       root = of_find_node_by_path("/");
-       if (!root)
-               return false;
+       while(match->compatible){
+               if(of_machine_is_compatible(match->compatible))
+                       return true;
+               match++;
+       }
 

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 33

Quoted 2 lines

drivers/soc/tegra/common.c +1 -1

-       return of_match_node(tegra_machine_match, root) != NULL;
+       return false;

Quoted 10 lines · Arnd Bergmann

Quoted 8 lines · Yangtao Li

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count.

Signed-off-by Yangtao Li <[email protected]>

---

Anders ran into a crash after this patch, on a non-tegra qemu platform:

[    0.055907] ASID allocator initialised with 32768 entries
[    0.063381] rcu: Hierarchical SRCU implementation.
[    0.137238] Unable to handle kernel paging request at virtual
address ffff00000935f0c0
[    0.137274] Mem abort info:
[    0.137291]   ESR = 0x96000007
[    0.137320]   Exception class = DABT (current EL), IL = 32 bits
[    0.137337]   SET = 0, FnV = 0
[    0.137352]   EA = 0, S1PTW = 0
[    0.137370] Data abort info:
[    0.137387]   ISV = 0, ISS = 0x00000007
[    0.137401]   CM = 0, WnR = 0
[    0.137456] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
[    0.137479] [ffff00000935f0c0] pgd=00000000bdfff003,
pud=00000000bdffe003, pmd=00000000bdffa003, pte=0000000000000000
[    0.137644] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[    0.137766] Modules linked in:
[    0.137950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.20.0-rc3-next-20181122-00006-g38d8a1f80349-dirty #2
[    0.137975] Hardware name: linux,dummy-virt (DT)
[    0.138071] pstate: 00000085 (nzcv daIf -PAN -UAO)
[    0.138110] pc : __of_device_is_compatible+0x30/0x138
[    0.138132] lr : of_device_is_compatible+0x40/0x68
[    0.138147] sp : ffff00000804bc80
[    0.138167] x29: ffff00000804bc80 x28: 0000000000000000
[    0.138207] x27: 0000000000000000 x26: 0000000000000000
[    0.138224] x25: ffff80007dfed2a8 x24: ffff000009301000
[    0.138239] x23: 0000000000000000 x22: 0000000000000000
[    0.138254] x21: ffff00000935f0c0 x20: ffff00000935f0c0
[    0.138269] x19: 0000000000000000 x18: 0000000000000400
[    0.138284] x17: 0000000000000000 x16: 0000000000000000
[    0.138298] x15: 0000000000000400 x14: 0000000000000400
[    0.138313] x13: 0000000000000000 x12: 0000000000000020
[    0.138327] x11: 0000000000000008 x10: 0101010101010101
[    0.138357] x9 : 6862726efffefeff x8 : 7f7f7f7f7f7f7f7f
[    0.138373] x7 : fefefefefefeff2e x6 : 0080808080808080
[    0.138387] x5 : 0000000000000002 x4 : 0000000000000001
[    0.138402] x3 : 0000000000000000 x2 : 0000000000000000
[    0.138416] x1 : ffff00000935f0c0 x0 : ffff80007dfed2a8
[    0.138475] Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____))
[    0.138540] Call trace:
[    0.138607]  __of_device_is_compatible+0x30/0x138
[    0.138632]  of_device_is_compatible+0x40/0x68
[    0.138654]  of_machine_is_compatible+0x34/0x68
[    0.138672]  soc_is_tegra+0x2c/0x40
[    0.138689]  tegra_flowctrl_init+0x28/0x108
[    0.138706]  do_one_initcall+0x5c/0x178
[    0.138722]  kernel_init_freeable+0xd0/0x240
[    0.138741]  kernel_init+0x10/0x108
[    0.138755]  ret_from_fork+0x10/0x18
[    0.138913] Code: b4000861 f90013b5 aa0103f5 52800013 (39400021)
[    0.139229] ---[ end trace d4d0fc77e9b04fa6 ]---
[    0.139448] note: swapper/0[1] exited with preempt_count 1
[    0.140598] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b
[    0.140767] ---[ end Kernel panic - not syncing: Attempted to kill

init! exitcode=0x0000000b ]---

I'm not completely sure what's wrong with the patch, but I assume it never worked on non-tegra machines.

Quoted 20 lines

Quoted 20 lines

drivers/soc/tegra/common.c +6 -4

index cd8f41351add..0b40700b672a 100644
@@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
 
 bool soc_is_tegra(void)
 {
-       struct device_node *root;
+       struct of_device_id *match = tegra_machine_match;
 
-       root = of_find_node_by_path("/");
-       if (!root)
-               return false;
+       while(match->compatible){
+               if(of_machine_is_compatible(match->compatible))
+                       return true;
+               match++;
+       }
 

Quoted 2 lines

drivers/soc/tegra/common.c static const struct of_device_id tegra_machine_match[] = { · line 33

Quoted 2 lines

drivers/soc/tegra/common.c +1 -1

-       return of_match_node(tegra_machine_match, root) != NULL;
+       return false;

I would also note that this is a rather inefficient way to check for a particular platform, as we to do the string search through the DT for each entry in the table now instead of doing it once.

Arnd