Hi,
Thanks for taking a look at this.
On 6/7/19 4:53 AM, Sudeep Holla wrote:
On Fri, May 03, 2019 at 06:24:05PM -0500, Jeremy Linton wrote:
The ACPI specification implies that the IDENTICAL flag should be
set on all non leaf nodes where the children are identical.
This means that we need to be searching for the last node with
the identical flag set rather than the first one.
Since this flag is also dependent on the table revision, we
need to add a bit of extra code to verify the table revision,
and the next node's state in the traversal. Since we want to
avoid function pointers here, lets just special case
the IDENTICAL flag.
Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
---
drivers/acpi/pptt.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 1865515297ca..456e1c0a35ae 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -432,17 +432,39 @@ static void cache_setup_acpi_cpu(struct acpi_table_header *table,
}
}
+static bool flag_identical(struct acpi_table_header *table_hdr,
+ struct acpi_pptt_processor *cpu)
Not sure if it's email client problem, but I see quite a few mis-alignment
with parenthesis like above one.
It looks fine in the original editor/text patch, but yes in my email
client I see it off by one (or two/three now that i'm replying). Its a
mix of tabs/spaces and I've seen this happen before in my email client
due to the leading "[>+]"?
+{
+ struct acpi_pptt_processor *next;
+
+ /* heterogeneous machines must use PPTT revision > 1 */
+ if (table_hdr->revision < 2)
+ return false;
+
+ /* Locate the last node in the tree with IDENTICAL set */
+ if (cpu->flags & ACPI_PPTT_ACPI_IDENTICAL) {
+ next = fetch_pptt_node(table_hdr, cpu->parent);
+ if (!(next && next->flags & ACPI_PPTT_ACPI_IDENTICAL))
+ return true;
+ }
+
+ return false;
+}
+
/* Passing level values greater than this will result in search termination */
#define PPTT_ABORT_PACKAGE 0xFF
-static struct acpi_pptt_processor *acpi_find_processor_package_id(struct acpi_table_header *table_hdr,
+static struct acpi_pptt_processor *acpi_find_processor_tag_id(struct acpi_table_header *table_hdr,
struct acpi_pptt_processor *cpu,
int level, int flag)
{
struct acpi_pptt_processor *prev_node;
while (cpu && level) {
- if (cpu->flags & flag)
+ if (flag == ACPI_PPTT_ACPI_IDENTICAL) {
flag_identical anyways check the flag, so I assume you can drop the above
check.
? I think that would be a bug because then we would always be returning
the value of the IDENTICAL flag rather than the other flags passed into
this routine. This is the special case I think Raphael was asking for
rather than the function pointer/callback interface.
+ if (flag_identical(table_hdr, cpu))
+ break;
+ } else if (cpu->flags & flag)
break;
pr_debug("level %d\n", level);
prev_node = fetch_pptt_node(table_hdr, cpu->parent);
@@ -480,7 +502,7 @@ static int topology_get_acpi_cpu_tag(struct acpi_table_header *table,
cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
if (cpu_node) {
- cpu_node = acpi_find_processor_package_id(table, cpu_node,
+ cpu_node = acpi_find_processor_tag_id(table, cpu_node,
level, flag);
Again misaligned, may be that's because of renaming.
--
Regards,
Sudeep
