Remove fatal error for non mobile platform#581
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR relaxes thermald’s startup behavior on non-mobile platforms by turning the ACPI PM profile “non-mobile” condition from a fatal startup blocker into a warning so users can run the daemon on more systems.
Changes:
- Downgrade the non-mobile ACPI PM profile check from fatal exit to a warning.
- Stop aborting engine initialization based on the ACPI PM profile check result (both default and adaptive engines).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/thd_engine.cpp | Logs a warning instead of exiting on non-mobile PM profiles; init path no longer returns fatal on this check. |
| src/thd_engine_adaptive.cpp | Removes fatal early-return on non-mobile PM profiles during adaptive engine init. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
262
to
266
| thd_log_info("PM profile is %s\n", pm_profile.c_str()); | ||
| if (pm_profile != "2" && pm_profile != "8") { | ||
| thd_log_error("Non mobile platform, exiting..\n"); | ||
| return THD_FATAL_ERROR; | ||
| thd_log_warn("Non mobile platform, thermal tables may not have been validated.\n"); | ||
| return THD_ERROR; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
I am thinking to turn this function to void
Comment on lines
+296
to
+300
| @@ -297,9 +297,7 @@ int cthd_engine::thd_engine_init(bool ignore_cpuid_check, bool adaptive) { | |||
| thd_log_debug("Ignore CPU ID check for MSRs\n"); | |||
| proc_list_matched = true; | |||
| } else { | |||
| if (check_acpi_platform_profile() != THD_SUCCESS) { | |||
| return THD_FATAL_ERROR; | |||
| } | |||
| check_acpi_platform_profile(); | |||
Contributor
Author
There was a problem hiding this comment.
With ignore_cpuid_check this function shouldn't be called?
spandruvada
force-pushed
the
thermald_next
branch
from
8ddc544 to
fd03d3f
Compare
|
|
||
| int cthd_engine::check_acpi_platform_profile() { | ||
| void cthd_engine::check_acpi_platform_profile() { | ||
| // Check PM profile and fail to start for non mobile platforms |
spandruvada
force-pushed
the
thermald_next
branch
from
fd03d3f to
23237ab
Compare
Comment on lines
306
to
309
| } | ||
|
|
||
| check_acpi_platform_profile(); | ||
| } |
| int cthd_engine::check_acpi_platform_profile() { | ||
| // Check PM profile and fail to start for non mobile platforms | ||
| void cthd_engine::check_acpi_platform_profile() { | ||
| // Check PM profile and warn to start for non mobile platforms |
Downgrade to warning as some user do want to run on non Mobile platforms. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
spandruvada
force-pushed
the
thermald_next
branch
from
23237ab to
de4821c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Downgrade to warning as some user do want to run on non Mobile platforms.