On 18/05/16 20:13, Prakash, Prashanth wrote:
On 5/18/2016 11:37 AM, Sudeep Holla wrote:
On 17/05/16 18:46, Prakash, Prashanth wrote:
Hi Sudeep,
On 5/11/2016 9:37 AM, Sudeep Holla wrote:
+
+static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
+{
+ int ret, i;
+ struct acpi_lpi_states_array *info;
+ struct acpi_device *d = NULL;
+ acpi_handle handle = pr->handle, pr_ahandle;
+ acpi_status status;
+
+ if (!osc_pc_lpi_support_confirmed)
+ return -EOPNOTSUPP;
+
+ max_leaf_depth = 0;
+ if (!acpi_has_method(handle, "_LPI"))
+ return -EINVAL;
+ flat_state_cnt = 0;
+
+ while (ACPI_SUCCESS(status = acpi_get_parent(handle, &pr_ahandle))) {
+ if (!acpi_has_method(handle, "_LPI"))
+ continue;
+
+ acpi_bus_get_device(handle, &d);
+ if (!strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
+ break;
+
+ max_leaf_depth++;
+ handle = pr_ahandle;
+ }
+
In the above loop, we break when we find a device with HID ==
ACPI_PROCESSOR_CONTAINER_HID. Shouldn't we continue to parse as long as the
parent HID == ACPI_PROCESSOR_CONTAINER_HID? This is required to make sure we
parse states in levels higher than cluster level in processor hierarchy.
Yes, thanks for pointing that out. With just clusters in _LPI on my dev
board, I missed it.
Same reason, I failed to notice it all this time :)
No worries.
Also, I think it might be safe to break out of the loop if we didn't find
_LPI package, instead of continuing. Given the presence of LPI entry:
"Enabled Parent State", I can't think of a non-ambiguous scenario where we
might find LPI packages in state N and N+2, but not in N+1, as we will not
be able to figure out which state in N enables which states in N+2.
Thoughts?
Though I admit I haven't thought in detail on how to deal with the
asymmetric topology, but that was the reason why I continue instead of
breaking.
Excerpts from the spec: "... This example is symmetric but that is not a
requirement. For example, a system may contain a different number of
processors in different containers or an asymmetric hierarchy where one
side of the topology tree is deeper than another...."
If it addresses asymmetric topology, sure we can keep as it doesn't impact other
scenarios. Also, we need to set handle=pr_ahandle prior to the continue statement.
Yes I noticed it yesterday, the more I think, I feel we can break out of
the loop. At any level, we need to have container nodes and that must
contain _LPI irrespective of asymmetricity. So you were right. I have
fixed accordingly and have pushed out on my branch.
--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
