Skip to content

thermald: spel: restrict PL init to XML-defined cdev only - #597

Merged
spandruvada merged 2 commits into
intel:masterfrom
priyjain1:spel_bug_fix
Sep 10, 2026
Merged

spandruvada merged 2 commits into
intel:masterfrom
priyjain1:spel_bug_fix

Conversation

@priyjain1

Copy link
Copy Markdown
Contributor

During thermald initialisation, read_cooling_devices() scans all qcom-spel:* nodes under /sys/class/powercap/ and creates a cthd_sysfs_cdev_spel object for every discovered domain/constraint combination. Because device_name is hardcoded to "SPEL" in the constructor, every object's update() call matches the single PPCC entry in the XML config and writes the PPCC-derived power limit to hardware — even for domains and constraints that the XML config never intended to manage.

Fix this by consulting the XML section before entering the SPEL discovery loop. A set of cdev type names that are explicitly listed in the config is built once; any discovered SPEL cdev whose generated type name (spel_controller__pl) is absent from that set is skipped before update() is called, preventing an unintended PL write to hardware.

When no SPEL cdevs are listed in the XML config the set is empty and the guard is not entered, preserving the existing behaviour.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts thermald’s Qualcomm SPEL cooling-device discovery so that initial power-limit programming (PPCC → HW writes via update()) only occurs for SPEL cooling devices explicitly named in the XML <CoolingDevices> configuration, avoiding unintended power-limit writes to unrelated power domains.

Changes:

  • Builds a set of XML-defined cooling-device type names before iterating SPEL sysfs nodes.
  • Generates per-domain/per-constraint SPEL type names (spel_controller_<domain>_pl<N>) and skips discovered SPEL nodes not present in the XML-defined set.
  • Adds <set> include to support the new lookup structure.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/thd_engine_default.cpp
@spandruvada

Copy link
Copy Markdown
Contributor

I prefer that the coplot suggestion is still signed off by you with assisted by tag same as Linux kernel uses.

Also I want to cut a release, you have anything urgent other than this change?

priyjain1 and others added 2 commits September 3, 2026 15:55
During thermald initialisation, read_cooling_devices() scans all
qcom-spel:* nodes under /sys/class/powercap/ and creates a
cthd_sysfs_cdev_spel object for every discovered domain/constraint
combination.  Because device_name is hardcoded to "SPEL" in the
constructor, every object's update() call matches the single PPCC
entry in the XML config and writes the PPCC-derived power limit to
hardware — even for domains and constraints that the XML config never
intended to manage.

Fix this by consulting the XML <CoolingDevices> section before
entering the SPEL discovery loop.  A set of cdev type names that are
explicitly listed in the config is built once; any discovered SPEL
cdev whose generated type name (spel_controller_<domain>_pl<N>) is
absent from that set is skipped before update() is called, preventing
an unintended PL write to hardware.

When no SPEL cdevs are listed in the XML config the set is empty and
the guard is not entered, preserving the existing behaviour.

Signed-off-by: Priyansh Jain <priyansh.jain@oss.qualcomm.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Priyansh Jain <priyansh.jain@oss.qualcomm.com>
@priyjain1

Copy link
Copy Markdown
Contributor Author

I prefer that the coplot suggestion is still signed off by you with assisted by tag same as Linux kernel uses.
Yes Done
Also I want to cut a release, you have anything urgent other than this change?
Yes, this will be very helpful. A significant number of changes have been added since the last revision, so it would be much easier to ask the distros to upgrade to the newer version rather than backport individual patches. I don't have any other urgent changes at the moment. Thank you.

@spandruvada
spandruvada merged commit 767cf1d into intel:master Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants