ARM: ACPI: Fix the helper which checks if only a /chosen node
PATCH 1/1 ARM: ACPI: Fix the helper which checks if only a /chosen node
From: Shannon Zhao <[email protected]>
Assuming that there are several other nodes and a /chosen node in a DTS and the /chosen node is last node, of_scan_flat_dt() will return 0 while we expect it returns 1 when it's used to call a function to check if there is only a /chosen node in DTS.
Fix this by passing a meaningful parameter instead of NULL to of_scan_flat_dt() to record if it finds a node that is not /chosen node.
--- arch/arm64/kernel/acpi.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
arch/arm64/kernel/acpi.c +11 -3
@@ -60,12 +60,15 @@ static int __init dt_scan_depth1_nodes(unsigned long node, const char *uname, int depth, void *data) { + bool *found = data; +/* * Return 1 as soon as we encounter a node at depth 1 that is * not the /chosen node. */ if (depth ==1 && (strcmp (uname,"chosen" ) !=0 )) - return 1; + *found = true; +return 0 ; }
@@ -176,15 +179,20 @@ out:*/ void __init acpi_boot_table_init (void ) { +bool found = false;/* * Enable ACPI instead of device tree unless * - ACPI has been disabled explicitly (acpi=off), or * - the device tree is not empty (it has more than just a /chosen node) * and ACPI has not been force enabled (acpi=force) */ -if (param_acpi_off || - (!param_acpi_force &&of_scan_flat_dt (dt_scan_depth1_nodes,NULL ))) +if (param_acpi_off) {return ; + }else if (!param_acpi_force) { +of_scan_flat_dt (dt_scan_depth1_nodes, &found); +if (found) +return ; + }/* * ACPI is disabled at this point. Enable it in order to parse
Signature · 1 line
2.0.4
PATCH 1/1 ARM: ACPI: Fix the helper which checks if only a /chosen node
Quoted 4 lines · Shannon Zhao
From: Shannon Zhao <[email protected]>
Assuming that there are several other nodes and a /chosen node in a DTS and the /chosen node is last node,
of_scan_flat_dt()will return 0
If those 'several other nodes' are not all subnodes of /chosen, at least one of them will be at depth 1, right? Could you give an example DTS where this goes wrong?
Quoted 4 lines
while
we expect it returns 1 when it's used to call a function to check if there is only a /chosen node in DTS.
of_scan_flat_dt() will terminate early and return 1 on the first occasion that the callback returns 1. The callback returns 1 if it encounters a node at depth 1 whose name is not 'chosen'. So I am failing to see how this code is broken.
Quoted 3 lines
Fix this by passing a meaningful parameter instead of NULL to
of_scan_flat_dt()to record if it finds a node that is not /chosen node.
While it is hardly a performance concern, you really don't need to traverse the entire FDT to figure out that /chosen is not the only depth 1 node.
Quoted 58 lines
Signed-off-by Shannon Zhao <[email protected]>--- arch/arm64/kernel/acpi.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)arch/arm64/kernel/acpi.c +11
-4@@ -60,12 +60,15 @@ static int __init dt_scan_depth1_nodes(unsigned long node, const char *uname, int depth, void *data) { + bool *found = data; +/* * Return 1 as soon as we encounter a node at depth 1 that is * not the /chosen node. */ if (depth ==1 && (strcmp (uname,"chosen" ) !=0 )) - return 1; + *found = true; +return 0 ; }@@ -176,15 +179,20 @@ out:*/ void __init acpi_boot_table_init (void ) { +bool found = false;/* * Enable ACPI instead of device tree unless * - ACPI has been disabled explicitly (acpi=off), or * - the device tree is not empty (it has more than just a /chosen node) * and ACPI has not been force enabled (acpi=force) */ -if (param_acpi_off || - (!param_acpi_force &&of_scan_flat_dt (dt_scan_depth1_nodes,NULL ))) +if (param_acpi_off) {return ; + }else if (!param_acpi_force) { +of_scan_flat_dt (dt_scan_depth1_nodes, &found); +if (found) +return ; + }/* * ACPI is disabled at this point. Enable it in order to parse -- 2.0.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 ARM: ACPI: Fix the helper which checks if only a /chosen node
Quoted 15 lines · Ard Biesheuvel
Quoted 4 lines · Shannon Zhao
From: Shannon Zhao <[email protected]>
Assuming that there are several other nodes and a /chosen node in a DTS and the /chosen node is last node,
of_scan_flat_dt()will return 0If those 'several other nodes' are not all subnodes of /chosen, at least one of them will be at depth 1, right? Could you give an example DTS where this goes wrong?
Quoted 4 lines
while
we expect it returns 1 when it's used to call a function to check if there is only a /chosen node in DTS.
Quoted 5 lines
of_scan_flat_dt()will terminate early and return 1 on the first occasion that the callback returns 1. The callback returns 1 if it encounters a node at depth 1 whose name is not 'chosen'. So I am failing to see how this code is broken.
Oh, I didn't see the !rc within the for() loop before. Sorry for the noise.
Signature · 1 line
Shannon