powercap: Add generic hierarchy support and convert DTPM
v4 cover powercap: Add generic hierarchy support and convert DTPM
Powercap controllers may expose trees of power zones, such as packages, clusters and CPUs. This series adds generic helpers to duplicate a static hierarchy description, create its zones through backend callbacks, and destroy them in reverse order. Parents must precede their children in the node array; the helpers validate that ordering rather than sorting nodes.
Drivers supporting multiple platforms can place each platform's hierarchy description and node array in initdata. powercap_hierarchy_dup() creates a working copy of the selected hierarchy and rebases its parent pointers. All original descriptions, including those for unused platforms, can then be reclaimed automatically with init memory. Names and backend-private data are not duplicated and must remain valid for the working hierarchy.
The controller supplies creation and destruction callbacks to connect the generic hierarchy traversal to its backend. The creation callback receives the control type, node name, private data and parent zone, registers the zone with its operations and constraints, and returns it to the helper. The destruction callback unregisters it during rollback or teardown.
The API supports both fatal creation errors, which roll back previously created zones, and optional nodes: a NULL callback result skips a node and its descendants while allowing independent branches to be created.
The helpers were motivated by the SPEL work and also let DTPM share the same hierarchy infrastructure:
https://lore.kernel.org/lkml/20260702-qcom_spel_driver_upstream-v3-0-434d50f0c5b0@oss.qualcomm.com/
A kselftest exercises static and duplicated hierarchies, skipped branches, reuse after partial destruction, and rejection of repeated creation. Its userspace script checks the 16 exported zones and selected attribute values, then removes the module with a sysfs attribute descriptor open.
DTPM is converted to the generic helpers while preserving best-effort creation when individual backends are unavailable. The CPU backend finds an online sibling when a hierarchy's representative CPU is offline. Entirely offline policies remain unavailable during initialization.
The RK3399 description is reduced to one CPU entry per cpufreq policy. The old backend already suppressed duplicate registration; the extra entries were redundant descriptions, not additional registered zones. Finally, a shared Rockchip virtual descriptor and an RK3588 CPU hierarchy are added. RK3588's three frequency domains are represented by cpu@0, cpu@400 and cpu@600 below a virtual package node; no GPU zone is added for RK3588 in this series.
Changes in v4 (since v3):
- Rebase on v7.3.
- Address the selftest issues reported by Sashiko:
- Add a kselftest config fragment requesting CONFIG_POWERCAP, CONFIG_MODULES and CONFIG_MODULE_UNLOAD, guard the module build with CONFIG_POWERCAP, and skip building when prerequisites or Module.symvers are missing instead of failing at modpost.
- Resolve the kernel output directory and pass absolute O=, KBUILD_OUTPUT and module-output paths to Kbuild so cleanup works when a relative O= is inherited from the parent selftest make invocation.
- Keep the statically initialized mutex in permanent module data so its lockdep key does not point into freed module init memory. Retain a separate init-only description for the hierarchy duplication test.
- Return KSFT_SKIP when powercap sysfs support or the test module is unavailable, while retaining failures for actual module-load errors.
Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Add POWERCAP_HIERARCHY_INIT() and document static versus duplicated hierarchy ownership and lock initialization.
- Explain how duplication retains the selected platform hierarchy while init memory containing the original descriptions is reclaimed, and describe the controller callbacks and their arguments.
- Reject populated hierarchies with -EBUSY before invoking callbacks.
- Validate names and parent ordering before zone creation, including for static hierarchies; document the accepted hierarchy structure.
- Use explicit mutex locking, a signed reverse-loop index, and the clearer powercap_node.zone member name.
- Define NULL creation results as skipped branches and retain rollback for error pointers, preserving DTPM's best-effort behavior.
- Handle offline CPU representatives through an online policy sibling while holding the CPU hotplug read lock.
- Fix selftest output-directory handling and module cleanup, use powercap-owned zone allocations, and keep shell validation/counting in the main shell so failures propagate.
- Extend and document selftest coverage for static creation, skipped branches, hierarchy reuse, -EBUSY, and an open descriptor at teardown.
- Fix indentation, __initdata placement, unnamed setup callback arguments, and whitespace-only lines reported by checkpatch and patch application.
- Correct descriptions of redundant CPU entries and actual sysfs checks.
Earlier generic hierarchy API and selftest submission:
https://lore.kernel.org/all/[email protected]/
Daniel Lezcano (6):
powercap: Add generic zone hierarchy creation helpers selftests/powercap: Add powercap hierarchy creation API tests
powercap: dtpm: Remove duplicate RK3399 CPU hierarchy nodes
powercap: dtpm: Use generic powercap hierarchy helpers
powercap: dtpm: Rename Rockchip virtual node descriptor
powercap: dtpm: Add RK3588 hierarchy
drivers/powercap/dtpm.c | 177 +++++----- drivers/powercap/dtpm_cpu.c | 62 +++- drivers/powercap/dtpm_devfreq.c | 22 +- drivers/powercap/powercap_sys.c | 204 ++++++++++++ drivers/soc/rockchip/dtpm.c | 129 +++++-- include/linux/dtpm.h | 6 +- include/linux/powercap.h | 205 ++++++++++++ tools/testing/selftests/Makefile | 1 + tools/testing/selftests/powercap/Kbuild | 5 + tools/testing/selftests/powercap/Makefile | 46 +++ tools/testing/selftests/powercap/config | 3 + .../selftests/powercap/powercap_hierarchy.c | 314 ++++++++++++++++++ .../selftests/powercap/powercap_hierarchy.sh | 128 +++++++ 13 files changed, 1150 insertions(+), 152 deletions(-) create mode 100644 tools/testing/selftests/powercap/Kbuild create mode 100644 tools/testing/selftests/powercap/Makefile create mode 100644 tools/testing/selftests/powercap/config create mode 100644 tools/testing/selftests/powercap/powercap_hierarchy.c create mode 100755 tools/testing/selftests/powercap/powercap_hierarchy.sh
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
Signature · 1 line
2.43.0
PATCH v4 1/6 powercap: Add generic zone hierarchy creation helpers
Powercap controllers may need to create several powercap zones organized as a hierarchy. At present, each controller has to open-code the hierarchy traversal, parent lookup, error rollback and reverse-order destruction.
Introduce struct powercap_hierarchy to describe a powercap hierarchy. Each node contains its name, parent, backend-specific data and the powercap zone created for it.
For example, the following description:
static struct powercap_node nodes[] = {
{ .name = "package" },
{ .name = "cpu", .parent = &nodes[0] },
{ .name = "gpu", .parent = &nodes[0] },};
static struct powercap_hierarchy hierarchy =
POWERCAP_HIERARCHY_INIT(hierarchy, nodes, ARRAY_SIZE(nodes));creates the following hierarchy:
package
|-- cpu
`-- gpu
Drivers supporting multiple platforms may provide a separate static hierarchy description for each platform. These descriptions and their node arrays can reside in initdata: only the hierarchy selected for the running platform needs to remain available after initialization.
Add powercap_hierarchy_dup() to turn the selected description into a working runtime hierarchy. Copy its node array and rebase parent pointers to the copy, allowing all original descriptions, including the unselected ones, to be reclaimed automatically with init memory. The names and backend-private data are not duplicated and must remain valid for the lifetime of the working hierarchy.
Add powercap_hierarchy_create() to walk the description in order. The hierarchy describes the topology, but only the controller knows how to initialize a zone for its underlying hardware and which powercap operations and constraints to register.
The controller therefore supplies a function of type powercap_node_create_t as the third argument to powercap_hierarchy_create().
For each node, the helper calls this function with the control type, node name, node-private data and the already-created parent zone, or NULL for a root. The function uses these arguments to initialize its backend state and call powercap_register_zone(), then returns the registered zone.
The controller also supplies a powercap_node_destroy_t function as the fourth argument. The helper calls it to unregister zones during rollback or destruction, passing the control type, zone and node-private data. These functions adapt the generic traversal to controller-specific zone creation and cleanup; they are separate from the powercap zone operations used for subsequent power readings and limit updates.
The creation function may let powercap allocate the zone or embed it in a backend-specific structure to retrieve private data later using container_of(). For example, an embedded zone can be allocated and registered inside this function as follows:
struct foo_powercap_zone {
struct powercap_zone zone;
struct foo_domain *domain;
};
foo_zone = kzalloc(sizeof(*foo_zone), GFP_KERNEL);
if (!foo_zone)
return ERR_PTR(-ENOMEM);
foo_zone->domain = domain;pcz = powercap_register_zone(&foo_zone->zone, pct, name, parent,
&foo_zone_ops, nr_constraints,
&foo_constraint_ops);For an embedded zone, its allocation, private state and lifetime remain under the control of the backend.
The creation callback returns the registered powercap_zone pointer. The hierarchy helper stores this pointer to provide it as the parent of subsequent nodes and to pass it back during hierarchy destruction.
Validate node names and require every parent to be an earlier entry in the same array before creating zones. Reject an already populated hierarchy with -EBUSY without modifying its existing zones. Provide POWERCAP_HIERARCHY_INIT() for static instances used directly by the creation and destruction helpers; duplication initializes its own lock.
A callback may return NULL to skip a node and its descendants while continuing independent branches. Error pointers abort creation and trigger rollback.
Add powercap_hierarchy_destroy() to destroy the hierarchy in reverse order, ensuring that children are removed before their parents. Use the same mechanism to roll back previously created zones when the creation of a subsequent node fails.
Serialize creation and destruction of each hierarchy to prevent concurrent updates of the powercap zone pointers stored in its runtime copy.
This provides a common mechanism for creating controller-defined powercap hierarchies while keeping controller-specific operations outside the powercap core.
---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.
Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Add a static initializer and clarify hierarchy ownership and locking.
- Check for existing zones and validate names and parent ordering before creation; use zone for the runtime pointer member.
- Allow NULL callback results to skip branches; keep errors fatal.
- Use explicit locking and a signed reverse-loop index.
- Explain the controller callbacks, their arguments and their relationship to zone operations; update the description.
drivers/powercap/powercap_sys.c | 204 +++++++++++++++++++++++++++++++ include/linux/powercap.h | 205 ++++++++++++++++++++++++++++++++ 2 files changed, 409 insertions(+)
drivers/powercap/powercap_sys.c +204 -0
@@ -667,6 +667,210 @@ int powercap_unregister_control_type(struct powercap_control_type *control_type) }EXPORT_SYMBOL_GPL (powercap_unregister_control_type); +struct powercap_hierarchy * +powercap_hierarchy_dup (const struct powercap_hierarchy *hierarchy) +{ +struct powercap_hierarchy *copy; +size_t i; + +if (!hierarchy || !hierarchy->nodes || !hierarchy->nr_nodes) +return ERR_PTR (-EINVAL ); + + copy =kzalloc_obj (*copy); +if (!copy) +return ERR_PTR (-ENOMEM ); + +/* +* The source node array may live in init memory. A shallow copy would +* leave parent pointers referencing that array after it has been freed. +* Copy the nodes now and rebase their parent pointers below. The name +* and data objects remain owned by the backend and are not duplicated. +*/ + copy->nodes =kmemdup_array (hierarchy->nodes, hierarchy->nr_nodes, +sizeof (*copy->nodes),GFP_KERNEL ); +if (!copy->nodes) { +kfree (copy); +return ERR_PTR (-ENOMEM ); + } + + copy->nr_nodes = hierarchy->nr_nodes; +mutex_init (©->lock); + +for (i =0 ; i < copy->nr_nodes; i++) { +const struct powercap_node *parent = hierarchy->nodes[i].parent; +ptrdiff_t index; + + copy->nodes[i].zone =NULL ; + +if (!copy->nodes[i].name) + goto invalid; + +if (!parent) +continue ; + + index = parent - hierarchy->nodes; +if (index <0 || index >= i) + goto invalid; + + copy->nodes[i].parent = ©->nodes[index]; + } + +return copy; + +invalid: +mutex_destroy (©->lock); +kfree (copy->nodes); +kfree (copy); + +return ERR_PTR (-EINVAL ); +} +EXPORT_SYMBOL_GPL (powercap_hierarchy_dup); + +void powercap_hierarchy_free (struct powercap_hierarchy *hierarchy) +{ +if (!hierarchy) +return ; + +mutex_destroy (&hierarchy->lock); +kfree (hierarchy->nodes); +kfree (hierarchy); +} +EXPORT_SYMBOL_GPL (powercap_hierarchy_free); + +static void __powercap_hierarchy_destroy (struct powercap_control_type *pct, +struct powercap_hierarchy *hierarchy, +powercap_node_destroy_t powercap_node_destroy) +{ +ssize_t i = (typeof (i ))hierarchy->nr_nodes -1 ; + +/* Creation validates that parents precede their children. */ +for (; i >=0 ; i--) { +if (!hierarchy->nodes[i].zone) +continue ; + +powercap_node_destroy (pct, hierarchy->nodes[i].zone, + hierarchy->nodes[i].data); + + hierarchy->nodes[i].zone =NULL ; + } +} + +int powercap_hierarchy_destroy (struct powercap_control_type *pct, +struct powercap_hierarchy *hierarchy, +powercap_node_destroy_t powercap_node_destroy) +{ +if (!pct || !hierarchy || !hierarchy->nodes || + !hierarchy->nr_nodes || !powercap_node_destroy) +return -EINVAL ; + +mutex_lock (&hierarchy->lock); + +__powercap_hierarchy_destroy (pct, hierarchy, powercap_node_destroy); + +mutex_unlock (&hierarchy->lock); + +return 0 ; +} +EXPORT_SYMBOL_GPL (powercap_hierarchy_destroy); + +/* +* Validate the hierarchy description before creating any zones. The caller +* must supply a non-empty node array and hold the hierarchy mutex. +* +* Every node must have a non-NULL name. A NULL parent identifies a root; +* otherwise, the parent must be an earlier entry in the same array. This +* excludes self-parenting, cycles and references outside the array, ensures +* parents are processed before children, and makes reverse-order destruction +* safe. Multiple roots are allowed; sibling ordering and unique names are not +* required. +* +* Return 0 for a valid description, or -EINVAL if any node violates these +* requirements. Runtime zone state is checked separately by the caller. +*/ +static int powercap_hierarchy_validate (const struct powercap_hierarchy *hierarchy) +{ +size_t i; + +for (i =0 ; i < hierarchy->nr_nodes; i++) { +const struct powercap_node *parent = hierarchy->nodes[i].parent; +size_t j; + +if (!hierarchy->nodes[i].name) +return -EINVAL ; + +if (!parent) +continue ; + +for (j =0 ; j < i; j++) { +if (parent == &hierarchy->nodes[j]) +break ; + } + +if (j == i) +return -EINVAL ; + } + +return 0 ; +} + +int powercap_hierarchy_create (struct powercap_control_type *pct, +struct powercap_hierarchy *hierarchy, +powercap_node_create_t powercap_node_create, +powercap_node_destroy_t powercap_node_destroy) +{ +struct powercap_zone *pcz; +int ret; +size_t i; + +if (!pct || !hierarchy || !hierarchy->nodes || !hierarchy->nr_nodes || + !powercap_node_create || !powercap_node_destroy) +return -EINVAL ; + +mutex_lock (&hierarchy->lock); + +for (i =0 ; i < hierarchy->nr_nodes; i++) { +if (hierarchy->nodes[i].zone) { + ret = -EBUSY ; + goto out_unlock; + } + } + + ret =powercap_hierarchy_validate (hierarchy); +if (ret) + goto out_unlock; + +for (i =0 ; i < hierarchy->nr_nodes; i++) { +struct powercap_zone *parent =NULL ; + +if (hierarchy->nodes[i].parent) { + parent = hierarchy->nodes[i].parent->zone; +if (!parent) +continue ; + } + + pcz =powercap_node_create (pct, hierarchy->nodes[i].name, + hierarchy->nodes[i].data, parent); +if (IS_ERR (pcz)) { + ret =PTR_ERR (pcz); + goto rollback; + } + + hierarchy->nodes[i].zone = pcz; + } + + ret =0 ; + goto out_unlock; + +rollback: +__powercap_hierarchy_destroy (pct, hierarchy, powercap_node_destroy); + +out_unlock: +mutex_unlock (&hierarchy->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (powercap_hierarchy_create); +static int __init powercap_init (void ) {int result;
include/linux/powercap.h +205 -0
215 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v4 1/6] powercap: Add generic zone hierarchy creation helpers
Quoted 8 lines · Daniel Lezcano
Powercap controllers may need to create several powercap zones organized as a hierarchy. At present, each controller has to open-code the hierarchy traversal, parent lookup, error rollback and reverse-order destruction.
Introduce struct
powercap_hierarchyto describe a powercap hierarchy. Each node contains its name, parent, backend-specific data and the powercap zone created for it.
[snip]
Quoted 18 lines
Cc Manaf Meethalavalappu Pallikunhi <[email protected]>Signed-off-by Daniel Lezcano <[email protected]>---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Add a static initializer and clarify hierarchy ownership and locking.
- Check for existing zones and validate names and parent ordering before creation; use zone for the runtime pointer member.
- Allow NULL callback results to skip branches; keep errors fatal.
- Use explicit locking and a signed reverse-loop index.
- Explain the controller callbacks, their arguments and their relationship to zone operations; update the description.
Quoted 4 lines
drivers/powercap/powercap_sys.c | 204 +++++++++++++++++++++++++++++++ include/linux/powercap.h | 205 ++++++++++++++++++++++++++++++++ 2 files changed, 409 insertions(+)
Re: [PATCH v4 1/6] powercap: Add generic zone hierarchy creation helpers
Hi Lukasz,
thanks for the review
Quoted 38 lines · Lukasz Luba
Quoted 8 lines · Daniel Lezcano
Powercap controllers may need to create several powercap zones organized as a hierarchy. At present, each controller has to open-code the hierarchy traversal, parent lookup, error rollback and reverse-order destruction.
Introduce struct
powercap_hierarchyto describe a powercap hierarchy. Each node contains its name, parent, backend-specific data and the powercap zone created for it.[snip]
Quoted 19 lines
Cc Manaf Meethalavalappu Pallikunhi <[email protected]>Signed-off-by Daniel Lezcano <[email protected]>---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Add a static initializer and clarify hierarchy ownership and locking.
- Check for existing zones and validate names and parent ordering before creation; use zone for the runtime pointer member.
- Allow NULL callback results to skip branches; keep errors fatal.
- Use explicit locking and a signed reverse-loop index.
- Explain the controller callbacks, their arguments and their relationship
to zone operations; update the description.Quoted 4 lines
drivers/powercap/powercap_sys.c| 204 +++++++++++++++++++++++++++++++ include/linux/powercap.h | 205 ++++++++++++++++++++++++++++++++2 files changed, 409 insertions(+)Reviewed-by Lukasz Luba <[email protected]>
PATCH v4 2/6 selftests/powercap: Add powercap hierarchy creation API tests
In reply to Daniel LezcanoThe powercap hierarchy API introduces generic helpers to duplicate, instantiate and destroy a complete powercap hierarchy from a static description.
Add a kselftest exercising this API. The test builds a synthetic powercap hierarchy composed of a package, three CPU clusters and twelve CPUs, registers it as a powercap control type and checks the zone count and selected sysfs attribute values. It also checks that the controller directory disappears when the module is unloaded.
Exercise direct static creation, duplication, skipped clusters and their descendants, reuse after partial destruction, and repeated creation returning -EBUSY without replacing existing zone pointers. Callback checks verify the controller, name and backend-private data supplied by the helpers.
The test consists of:
* a kernel module implementing a synthetic powercap hierarchy and dummy callbacks;
* a userspace kselftest script that loads the module, checks 16 zones and their power and constraint attributes, unloads the module with an attribute descriptor open, and checks controller removal.
Use powercap-owned zone allocations so registration failures are cleaned up by the core. Build generated module files in the selftest output directory and extend the common clean target with Kbuild module cleanup.
Provide a config fragment for powercap and module support. Skip module builds when the configuration prerequisites or Module.symvers are missing, and skip runtime checks when powercap or the test module is unavailable. Resolve inherited kernel output paths before invoking Kbuild so cleanup also works with a relative O= argument.
Keep the statically initialized mutex in permanent module data so its lockdep key survives initialization. Use a separate init-only hierarchy description to exercise duplication and release the original node array.
This provides a regression test for the powercap hierarchy helpers and their integration with the powercap core.
---
Changes in v4 (since v3):
- Rebase on v7.3 and address the issues reported by Sashiko.
- Add a config fragment requesting powercap, modules and module unloading; guard the Kbuild module target with CONFIG_POWERCAP.
- Skip module builds when configuration prerequisites or Module.symvers are missing, avoiding unresolved-symbol failures during modpost.
- Resolve inherited output paths and pass absolute O=, KBUILD_OUTPUT and module-output paths to Kbuild, including during relative-O= cleanup.
- Keep the static mutex in permanent module data so its lockdep key survives init-memory reclamation; duplicate a separate init-only hierarchy description.
- Return KSFT_SKIP when powercap sysfs support or the module is absent.
- Update the commit description to explain these fixes.
Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Use TEST_GEN_FILES and MO=$(OUTPUT); preserve the common clean recipe.
- Use powercap-owned allocations for consistent registration cleanup.
- Keep validation and counting in the main shell so failures propagate.
- Add static creation, skipped branches, reuse and -EBUSY checks, and hold a sysfs descriptor open across module removal.
- Document the tests and describe their actual assertions.
- Fix closing-brace indentation and __initdata placement.
tools/testing/selftests/Makefile | 1 + tools/testing/selftests/powercap/Kbuild | 5 + tools/testing/selftests/powercap/Makefile | 46 +++ tools/testing/selftests/powercap/config | 3 + .../selftests/powercap/powercap_hierarchy.c | 314 ++++++++++++++++++ .../selftests/powercap/powercap_hierarchy.sh | 128 +++++++ 6 files changed, 497 insertions(+) create mode 100644 tools/testing/selftests/powercap/Kbuild create mode 100644 tools/testing/selftests/powercap/Makefile create mode 100644 tools/testing/selftests/powercap/config create mode 100644 tools/testing/selftests/powercap/powercap_hierarchy.c create mode 100755 tools/testing/selftests/powercap/powercap_hierarchy.sh
tools/testing/selftests/Makefile +1 -0
@@ -104,6 +104,7 @@ TARGETS += pidfd TARGETS += pid_namespace TARGETS += pipe TARGETS += power_supply +TARGETS += powercap TARGETS += powerpc TARGETS += prctl TARGETS += proc
tools/testing/selftests/powercap/Kbuild new file +5 -0
@@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 + +ifeq ($(CONFIG_POWERCAP),y) +obj-m += powercap_hierarchy.o +endif
tools/testing/selftests/powercap/Makefile new file +46 -0
46 lines not on this page · raw message
tools/testing/selftests/powercap/config new file +3 -0
@@ -0,0 +1,3 @@ +CONFIG_POWERCAP=y +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y
tools/testing/selftests/powercap/powercap_hierarchy.c new file +314 -0
314 lines not on this page · raw message
tools/testing/selftests/powercap/powercap_hierarchy.sh new file +128 -0
128 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v4 3/6 powercap: dtpm: Remove duplicate RK3399 CPU hierarchy nodes
In reply to Daniel LezcanoThe DTPM CPU backend registers one powercap zone per cpufreq policy, not per CPU. On RK3399, CPUs sharing the same cpufreq policy therefore map to the same DTPM zone.
Remove the redundant CPU nodes from the RK3399 hierarchy and keep only one CPU node per cpufreq policy. The backend already suppresses duplicate registration through its per-CPU state, so the removed entries are redundant descriptions of the same zones. Keep both CPU policy domains and the GPU in the hierarchy, preparing for one created zone per generic hierarchy node.
Also fix indentation in the Rockchip device match table.
---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.
Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Fix Rockchip match-table indentation here rather than in patch 1.
- Clarify that the existing backend suppresses duplicate registration.
drivers/soc/rockchip/dtpm.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)
drivers/soc/rockchip/dtpm.c +5 -17
@@ -20,30 +20,18 @@ static struct dtpm_node __initdata rk3399_hierarchy[] = { [2 ] = { .name ="/cpus/cpu@0" , .type =DTPM_NODE_DT , .parent = &rk3399_hierarchy[1] }, - [3 ] = { .name ="/cpus/cpu@1" , + [3 ] = { .name ="/cpus/cpu@100" , .type =DTPM_NODE_DT , .parent = &rk3399_hierarchy[1] }, - [4 ] = { .name ="/cpus/cpu@2" , + [4 ] = { .name ="/gpu@ff9a0000" , .type =DTPM_NODE_DT , .parent = &rk3399_hierarchy[1] }, - [5 ] = { .name ="/cpus/cpu@3" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [6 ] = { .name ="/cpus/cpu@100" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [7 ] = { .name ="/cpus/cpu@101" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [8 ] = { .name ="/gpu@ff9a0000" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [9 ] = {/* sentinel */ } + [5 ] = {/* sentinel */ } };static struct of_device_id __initdata rockchip_dtpm_match_table[] = { - { .compatible ="rockchip,rk3399" , .data = rk3399_hierarchy }, - {}, + { .compatible ="rockchip,rk3399" , .data = rk3399_hierarchy }, + {}, };static int __init rockchip_dtpm_init (void )
Signature · 1 line
2.43.0
Re: [PATCH v4 3/6] powercap: dtpm: Remove duplicate RK3399 CPU hierarchy nodes
Quoted 56 lines · Daniel Lezcano
The DTPM CPU backend registers one powercap zone per cpufreq policy, not per CPU. On RK3399, CPUs sharing the same cpufreq policy therefore map to the same DTPM zone.
Remove the redundant CPU nodes from the RK3399 hierarchy and keep only one CPU node per cpufreq policy. The backend already suppresses duplicate registration through its per-CPU state, so the removed entries are redundant descriptions of the same zones. Keep both CPU policy domains and the GPU in the hierarchy, preparing for one created zone per generic hierarchy node.
Also fix indentation in the Rockchip device match table.
Signed-off-by Daniel Lezcano <[email protected]>---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Fix Rockchip match-table indentation here rather than in patch 1.
- Clarify that the existing backend suppresses duplicate registration.drivers/soc/rockchip/dtpm.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)drivers/soc/rockchip/dtpm.c +3
-15@@ -20,30 +20,18 @@ static struct dtpm_node __initdata rk3399_hierarchy[] = { [2 ] = { .name ="/cpus/cpu@0" , .type =DTPM_NODE_DT , .parent = &rk3399_hierarchy[1] }, - [3 ] = { .name ="/cpus/cpu@1" , + [3 ] = { .name ="/cpus/cpu@100" , .type =DTPM_NODE_DT , .parent = &rk3399_hierarchy[1] }, - [4 ] = { .name ="/cpus/cpu@2" , + [4 ] = { .name ="/gpu@ff9a0000" , .type =DTPM_NODE_DT , .parent = &rk3399_hierarchy[1] }, - [5 ] = { .name ="/cpus/cpu@3" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [6 ] = { .name ="/cpus/cpu@100" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [7 ] = { .name ="/cpus/cpu@101" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [8 ] = { .name ="/gpu@ff9a0000" , - .type =DTPM_NODE_DT , - .parent = &rk3399_hierarchy[1] }, - [9 ] = {/* sentinel */ } + [5 ] = {/* sentinel */ }
drivers/soc/rockchip/dtpm.c static struct dtpm_node __initdata rk3399_hierarchy[] = { · line 37
Quoted 10 lines
drivers/soc/rockchip/dtpm.c +2
-2};static struct of_device_id __initdata rockchip_dtpm_match_table[] = { - { .compatible ="rockchip,rk3399" , .data = rk3399_hierarchy }, - {}, + { .compatible ="rockchip,rk3399" , .data = rk3399_hierarchy }, + {}, };static int __init rockchip_dtpm_init (void )
PATCH v4 4/6 powercap: dtpm: Use generic powercap hierarchy helpers
In reply to Daniel LezcanoDTPM implements its own hierarchy creation and destruction logic while the powercap core now provides generic helpers for this purpose.
Convert DTPM to use the generic powercap hierarchy infrastructure. Duplicate the platform hierarchy before creating it, and use powercap_hierarchy_create() and powercap_hierarchy_destroy() to manage the DTPM zones.
Add DTPM callbacks to create and destroy the zones associated with each powercap hierarchy node. Adapt the CPU and devfreq backends to return the created DTPM instance and use the node name provided by the hierarchy.
Convert the Rockchip RK3399 description to struct powercap_hierarchy and keep the device tree path as DTPM-specific node data.
This removes the DTPM-specific hierarchy traversal and destruction code and leaves the hierarchy management to the powercap core.
Preserve best-effort creation: an unavailable backend returns NULL to skip its branch, and setup failures are warned about and skipped rather than rolling back independent zones. Invalid node descriptors remain fatal.
When the representative CPU is offline, find an active policy through an online sibling whose related_cpus mask contains it. Hold the CPU hotplug read lock during selection and setup, and initialize the zone through the policy's active CPU while keeping the hierarchy-provided name. An entirely offline policy still has no active CPU to initialize through.
---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.
Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Preserve best-effort branch creation using the generic NULL result.
- Find an online policy sibling for offline CPU representatives and protect policy selection and setup against CPU hotplug.
- Keep generic API behavior and documentation in patch 1.
- Fix __initdata placement; match-table indentation is handled in patch 3.
- Name setup callback arguments and remove whitespace-only lines.
- Update the description.
drivers/powercap/dtpm.c | 177 +++++++++++++++----------------- drivers/powercap/dtpm_cpu.c | 62 ++++++++--- drivers/powercap/dtpm_devfreq.c | 22 ++-- drivers/soc/rockchip/dtpm.c | 67 +++++++++--- include/linux/dtpm.h | 6 +- 5 files changed, 195 insertions(+), 139 deletions(-)
drivers/powercap/dtpm.c +81 -96
@@ -35,6 +35,7 @@ static const char *constraint_name[] = {static DEFINE_MUTEX (dtpm_lock);static struct powercap_control_type *pct; +static struct powercap_hierarchy *dtpm_hierarchy;static struct dtpm *root;static int get_time_window_us (struct powercap_zone *pcz,int cid,u64 *window)
@@ -191,7 +192,7 @@ static int get_power_limit_uw(struct powercap_zone *pcz, int cid, u64 *power_limit) { *power_limit =to_dtpm (pcz)->power_limit; - +return 0 ; }
@@ -412,8 +413,7 @@ int dtpm_register(const char *name, struct dtpm *dtpm, struct dtpm *parent)return 0 ; } -static struct dtpm *dtpm_setup_virtual (const struct dtpm_node *hierarchy, -struct dtpm *parent) +static struct dtpm *dtpm_setup_virtual (const char *name,struct dtpm *parent) {struct dtpm *dtpm;int ret;
@@ -423,10 +423,9 @@ static struct dtpm *dtpm_setup_virtual(const struct dtpm_node *hierarchy,return ERR_PTR (-ENOMEM );dtpm_init (dtpm,NULL ); - ret =dtpm_register (hierarchy->name, dtpm, parent); + ret =dtpm_register (name, dtpm, parent);if (ret) { -pr_err ("Failed to register dtpm node '%s': %d\n" , - hierarchy->name, ret); +pr_err ("Failed to register dtpm node '%s': %d\n" , name, ret);kfree (dtpm);return ERR_PTR (ret); }
@@ -434,90 +433,88 @@ static struct dtpm *dtpm_setup_virtual(const struct dtpm_node *hierarchy,return dtpm; } -static struct dtpm *dtpm_setup_dt (const struct dtpm_node *hierarchy, +static struct dtpm *dtpm_setup_dt (const char *name,const char *path,struct dtpm *parent) {struct device_node *np; -int i, ret; +struct dtpm *dtpm =NULL ; +int i; - np =of_find_node_by_path (hierarchy->name); + np =of_find_node_by_path (path);if (!np) { -pr_err ("Failed to find '%s'\n" , hierarchy->name); +pr_err ("Failed to find '%s'\n" , path);return ERR_PTR (-ENXIO ); }for (i =0 ; i <ARRAY_SIZE (dtpm_subsys); i++) { +struct dtpm *tmp;if (!dtpm_subsys[i]->setup)continue ; - ret = dtpm_subsys[i]->setup (parent, np); -if (ret) { -pr_err ("Failed to setup '%s': %d\n" , dtpm_subsys[i]->name, ret); -of_node_put (np); -return ERR_PTR (ret); + tmp = dtpm_subsys[i]->setup (parent, np, name); +if (IS_ERR (tmp)) { +pr_err ("Failed to setup '%s': %ld\n" , + dtpm_subsys[i]->name,PTR_ERR (tmp)); + dtpm = tmp; +break ; + } + +if (tmp) { + dtpm = tmp; +break ; } }of_node_put (np); -/* -* By returning a NULL pointer, we let know the caller there -* is no child for us as we are a leaf of the tree -*/ -return NULL ; +return dtpm; } -typedef struct dtpm * (*dtpm_node_callback_t )(const struct dtpm_node *,struct dtpm *); - -static dtpm_node_callback_t dtpm_node_callback[] = { - [DTPM_NODE_VIRTUAL ] = dtpm_setup_virtual, - [DTPM_NODE_DT ] = dtpm_setup_dt, -}; - -static int dtpm_for_each_child (const struct dtpm_node *hierarchy, -const struct dtpm_node *it,struct dtpm *parent) +static struct powercap_zone * +dtpm_node_create (struct powercap_control_type *pct,const char *name, +void *data,struct powercap_zone *parent) { +struct dtpm_node *node = data; +struct dtpm *dtpm_parent = parent ?to_dtpm (parent) :NULL ;struct dtpm *dtpm; -int i, ret; -for (i =0 ; hierarchy[i].name; i++) { +if (!node) +return ERR_PTR (-EINVAL ); + +switch (node->type) { +case DTPM_NODE_VIRTUAL : + dtpm =dtpm_setup_virtual (name, dtpm_parent); +break ; +case DTPM_NODE_DT : + dtpm =dtpm_setup_dt (name, node->path, dtpm_parent); +break ; +default : +return ERR_PTR (-EINVAL ); + } -if (hierarchy[i].parent != it) -continue ; +/* An unavailable backend must not prevent independent branches. */ +if (IS_ERR (dtpm)) { +pr_warn ("Failed to create '%s' in the hierarchy: %ld\n" , + name,PTR_ERR (dtpm)); +return NULL ; + } - dtpm = dtpm_node_callback[hierarchy[i].type](&hierarchy[i], parent); +if (!dtpm) +return NULL ; -/* -* A NULL pointer means there is no children, hence we -* continue without going deeper in the recursivity. -*/ -if (!dtpm) -continue ; +return &dtpm->zone; +} -/* -* There are multiple reasons why the callback could -* fail. The generic glue is abstracting the backend -* and therefore it is not possible to report back or -* take a decision based on the error. In any case, -* if this call fails, it is not critical in the -* hierarchy creation, we can assume the underlying -* service is not found, so we continue without this -* branch in the tree but with a warning to log the -* information the node was not created. -*/ -if (IS_ERR (dtpm)) { -pr_warn ("Failed to create '%s' in the hierarchy\n" , - hierarchy[i].name); -continue ; - } +static void dtpm_node_destroy (struct powercap_control_type *pct, +struct powercap_zone *zone,void *data) +{ +struct dtpm *dtpm =to_dtpm (zone); - ret =dtpm_for_each_child (hierarchy, &hierarchy[i], dtpm); -if (ret) -return ret; - } +if (dtpm == root) + root =NULL ; -return 0 ; +dtpm_unregister (dtpm); }/**
@@ -528,27 +525,16 @@ static int dtpm_for_each_child(const struct dtpm_node *hierarchy,* description of the different node in the hierarchy. It creates the * tree in the sysfs filesystem under the powercap dtpm entry. * -* The expected tree has the format: -* -* struct dtpm_node hierarchy[] = { -* [0] { .name = "topmost", type = DTPM_NODE_VIRTUAL }, -* [1] { .name = "package", .type = DTPM_NODE_VIRTUAL, .parent = &hierarchy[0] }, -* [2] { .name = "/cpus/cpu0", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, -* [3] { .name = "/cpus/cpu1", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, -* [4] { .name = "/cpus/cpu2", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, -* [5] { .name = "/cpus/cpu3", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, -* [6] { } -* }; -* -* The last element is always an empty one and marks the end of the -* array. +* The platform description is a struct powercap_hierarchy. The private +* data associated with each powercap node describes how DTPM creates the +* corresponding zone. * * Return: zero on success, a negative value in case of error. Errors * are reported back from the underlying functions. */ int dtpm_create_hierarchy (struct of_device_id *dtpm_match_table) { -const struct dtpm_node *hierarchy; +const struct powercap_hierarchy *hierarchy;int i, ret;mutex_lock (&dtpm_lock);
@@ -571,10 +557,18 @@ int dtpm_create_hierarchy(struct of_device_id *dtpm_match_table) goto out_err; } - ret =dtpm_for_each_child (hierarchy,NULL ,NULL ); -if (ret) + dtpm_hierarchy =powercap_hierarchy_dup (hierarchy); +if (IS_ERR (dtpm_hierarchy)) { + ret =PTR_ERR (dtpm_hierarchy); + dtpm_hierarchy =NULL ; goto out_err; - + } + + ret =powercap_hierarchy_create (pct, dtpm_hierarchy, + dtpm_node_create, dtpm_node_destroy); +if (ret) + goto out_free_hierarchy; +for (i =0 ; i <ARRAY_SIZE (dtpm_subsys); i++) {if (!dtpm_subsys[i]->init)
@@ -590,31 +584,20 @@ int dtpm_create_hierarchy(struct of_device_id *dtpm_match_table)return 0 ; +out_free_hierarchy: +powercap_hierarchy_free (dtpm_hierarchy); + dtpm_hierarchy =NULL ; out_err:powercap_unregister_control_type (pct); out_pct: pct =NULL ; out_unlock:mutex_unlock (&dtpm_lock); - +return ret; }EXPORT_SYMBOL_GPL (dtpm_create_hierarchy); -static void __dtpm_destroy_hierarchy (struct dtpm *dtpm) -{ -struct dtpm *child, *aux; - -list_for_each_entry_safe (child, aux, &dtpm->children, sibling) -__dtpm_destroy_hierarchy (child); - -/* -* At this point, we know all children were removed from the -* recursive call before -*/ -dtpm_unregister (dtpm); -} -void dtpm_destroy_hierarchy (void ) {int i;
@@ -624,8 +607,8 @@ void dtpm_destroy_hierarchy(void)if (!pct) goto out_unlock; -__dtpm_destroy_hierarchy (root); - +powercap_hierarchy_destroy (pct, dtpm_hierarchy, dtpm_node_destroy); +for (i =0 ; i <ARRAY_SIZE (dtpm_subsys); i++) {
@@ -635,8 +618,10 @@ void dtpm_destroy_hierarchy(void) dtpm_subsys[i]->exit (); } +powercap_hierarchy_free (dtpm_hierarchy);powercap_unregister_control_type (pct); + dtpm_hierarchy =NULL ; pct =NULL ; root =NULL ;
drivers/powercap/dtpm_cpu.c +49 -13
110 lines not on this page · raw message
drivers/powercap/dtpm_devfreq.c +12 -10
@@ -144,7 +144,8 @@ static struct dtpm_ops dtpm_ops = { .release = pd_release, }; -static int __dtpm_devfreq_setup (struct devfreq *devfreq,struct dtpm *parent) +static struct dtpm *__dtpm_devfreq_setup (struct devfreq *devfreq, +struct dtpm *parent,const char *name) {struct device *dev = devfreq->dev.parent;struct dtpm_devfreq *dtpm_devfreq;
@@ -156,23 +157,23 @@ static int __dtpm_devfreq_setup(struct devfreq *devfreq, struct dtpm *parent) ret =dev_pm_opp_of_register_em (dev,NULL );if (ret) {pr_err ("No energy model available for '%s'\n" ,dev_name (dev)); -return -EINVAL ; +return ERR_PTR (-EINVAL ); } } dtpm_devfreq =kzalloc_obj (*dtpm_devfreq);if (!dtpm_devfreq) -return -ENOMEM ; +return ERR_PTR (-ENOMEM );dtpm_init (&dtpm_devfreq->dtpm, &dtpm_ops); dtpm_devfreq->devfreq = devfreq; - ret =dtpm_register (dev_name (dev), &dtpm_devfreq->dtpm, parent); + ret =dtpm_register (name, &dtpm_devfreq->dtpm, parent);if (ret) {pr_err ("Failed to register '%s': %d\n" ,dev_name (dev), ret);kfree (dtpm_devfreq); -return ret; +return ERR_PTR (ret); } ret = dev_pm_qos_add_request(dev, &dtpm_devfreq->qos_req,
@@ -185,23 +186,24 @@ static int __dtpm_devfreq_setup(struct devfreq *devfreq, struct dtpm *parent)dtpm_update_power (&dtpm_devfreq->dtpm); -return 0 ; +return &dtpm_devfreq->dtpm; out_dtpm_unregister:dtpm_unregister (&dtpm_devfreq->dtpm); -return ret; +return ERR_PTR (ret); } -static int dtpm_devfreq_setup (struct dtpm *dtpm,struct device_node *np) +static struct dtpm *dtpm_devfreq_setup (struct dtpm *dtpm,struct device_node *np, +const char *name) {struct devfreq *devfreq; devfreq =devfreq_get_devfreq_by_node (np);if (IS_ERR (devfreq)) -return 0 ; +return NULL ; -return __dtpm_devfreq_setup (devfreq, dtpm); +return __dtpm_devfreq_setup (devfreq, dtpm, name); }struct dtpm_subsys_ops dtpm_devfreq_ops = {
drivers/soc/rockchip/dtpm.c +50 -17
76 lines not on this page · raw message
include/linux/dtpm.h +3 -3
@@ -38,7 +38,8 @@ struct dtpm_subsys_ops {const char *name;int (*init)(void );void (*exit)(void ); -int (*setup)(struct dtpm *,struct device_node *); +struct dtpm *(*setup)(struct dtpm *parent,struct device_node *np, +const char *name); };enum DTPM_NODE_TYPE {
@@ -48,8 +49,7 @@ enum DTPM_NODE_TYPE {struct dtpm_node {enum DTPM_NODE_TYPE type; -const char *name; -struct dtpm_node *parent; +const char *path; };static inline struct dtpm *to_dtpm (struct powercap_zone *zone)
Signature · 1 line
2.43.0
Re: [PATCH v4 4/6] powercap: dtpm: Use generic powercap hierarchy helpers
Hi Daniel,
Quoted 45 lines · Daniel Lezcano
DTPM implements its own hierarchy creation and destruction logic while the powercap core now provides generic helpers for this purpose.
Convert DTPM to use the generic powercap hierarchy infrastructure. Duplicate the platform hierarchy before creating it, and use
powercap_hierarchy_create()andpowercap_hierarchy_destroy()to manage the DTPM zones.Add DTPM callbacks to create and destroy the zones associated with each powercap hierarchy node. Adapt the CPU and devfreq backends to return the created DTPM instance and use the node name provided by the hierarchy.
Convert the Rockchip RK3399 description to struct
powercap_hierarchyand keep the device tree path as DTPM-specific node data.This removes the DTPM-specific hierarchy traversal and destruction code and leaves the hierarchy management to the powercap core.
Preserve best-effort creation: an unavailable backend returns NULL to skip its branch, and setup failures are warned about and skipped rather than rolling back independent zones. Invalid node descriptors remain fatal.
When the representative CPU is offline, find an active policy through an online sibling whose
related_cpusmask contains it. Hold the CPU hotplug read lock during selection and setup, and initialize the zone through the policy's active CPU while keeping the hierarchy-provided name. An entirely offline policy still has no active CPU to initialize through.Signed-off-by Daniel Lezcano <[email protected]>---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Preserve best-effort branch creation using the generic NULL result.
- Find an online policy sibling for offline CPU representatives and protect policy selection and setup against CPU hotplug.
- Keep generic API behavior and documentation in patch 1.
- Fix__initdataplacement; match-table indentation is handled in patch 3.
- Name setup callback arguments and remove whitespace-only lines.
- Update the description.
Quoted 6 lines
drivers/powercap/dtpm.c | 177 +++++++++++++++----------------- drivers/powercap/dtpm_cpu.c | 62 ++++++++--- drivers/powercap/dtpm_devfreq.c | 22 ++-- drivers/soc/rockchip/dtpm.c | 67 +++++++++--- include/linux/dtpm.h | 6 +- 5 files changed, 195 insertions(+), 139 deletions(-)
Tha patch in general looks good. I only have one comment related to the future usage of this mechanism. For this implementation it's safe, but I would add some extra safety net around the of_find_node_by_path(). See comments below.
[snip]
drivers/powercap/dtpm.c static struct dtpm *dtpm_setup_virtual(const struct dtpm_node *hierarchy, · line 443
Quoted 11 lines
drivers/powercap/dtpm.c +3
-3-static struct dtpm *dtpm_setup_dt (const struct dtpm_node *hierarchy, +static struct dtpm *dtpm_setup_dt (const char *name,const char *path,struct dtpm *parent) {struct device_node *np; -int i, ret; +struct dtpm *dtpm =NULL ; +int i; - np =of_find_node_by_path (hierarchy->name);
I would add a NULL check for 'path' here, or below where dtpm_setup_dt() is called.
Usually it's up the the code which is calling that API to make sure that the NULL is not sneaked in. We probably can force drivers code for that, right?
Quoted 59 lines
drivers/powercap/dtpm.c +20
-21+ np =of_find_node_by_path (path);if (!np) { -pr_err ("Failed to find '%s'\n" , hierarchy->name); +pr_err ("Failed to find '%s'\n" , path);return ERR_PTR (-ENXIO ); }for (i =0 ; i <ARRAY_SIZE (dtpm_subsys); i++) { +struct dtpm *tmp;if (!dtpm_subsys[i]->setup)continue ; - ret = dtpm_subsys[i]->setup (parent, np); -if (ret) { -pr_err ("Failed to setup '%s': %d\n" , dtpm_subsys[i]->name, ret); -of_node_put (np); -return ERR_PTR (ret); + tmp = dtpm_subsys[i]->setup (parent, np, name); +if (IS_ERR (tmp)) { +pr_err ("Failed to setup '%s': %ld\n" , + dtpm_subsys[i]->name,PTR_ERR (tmp)); + dtpm = tmp; +break ; + } + +if (tmp) { + dtpm = tmp; +break ; } }of_node_put (np); -/* -* By returning a NULL pointer, we let know the caller there -* is no child for us as we are a leaf of the tree -*/ -return NULL ; +return dtpm; } -typedef struct dtpm * (*dtpm_node_callback_t )(const struct dtpm_node *,struct dtpm *); - -static dtpm_node_callback_t dtpm_node_callback[] = { - [DTPM_NODE_VIRTUAL ] = dtpm_setup_virtual, - [DTPM_NODE_DT ] = dtpm_setup_dt, -}; - -static int dtpm_for_each_child (const struct dtpm_node *hierarchy, -const struct dtpm_node *it,struct dtpm *parent) +static struct powercap_zone * +dtpm_node_create (struct powercap_control_type *pct,const char *name, +void *data,struct powercap_zone *parent) { +struct dtpm_node *node = data; +struct dtpm *dtpm_parent = parent ?to_dtpm (parent) :NULL ;struct dtpm *dtpm; -int i, ret;
drivers/powercap/dtpm.c static struct dtpm *dtpm_setup_virtual(const struct dtpm_node *hierarchy, · line 489
Quoted 10 lines
drivers/powercap/dtpm.c +8
-1-for (i =0 ; hierarchy[i].name; i++) { +if (!node) +return ERR_PTR (-EINVAL ); + +switch (node->type) { +case DTPM_NODE_VIRTUAL : + dtpm =dtpm_setup_virtual (name, dtpm_parent); +break ; +case DTPM_NODE_DT :
... or a NULL check here.
drivers/powercap/dtpm.c static struct dtpm *dtpm_setup_virtual(const struct dtpm_node *hierarchy, · line 494
Quoted 5 lines
drivers/powercap/dtpm.c +5
-0+ dtpm =dtpm_setup_dt (name, node->path, dtpm_parent); +break ; +default : +return ERR_PTR (-EINVAL ); + }
The rest looks good.
If we are sure we would never experience that NULL ptr coming to those registration callbacks - then OK.
Regards,
Lukasz
PATCH v4 5/6 powercap: dtpm: Rename Rockchip virtual node descriptor
In reply to Daniel LezcanoThe virtual DTPM node descriptor is named after the RK3399 even though it does not contain any SoC-specific information.
Rename it to rockchip_virtual so it can be reused by other Rockchip SoC hierarchies.
No functional change intended.
---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.
Changes in v3 (since v2):
- Rebase on v7.3-rc7 and refresh context after patch 4 style cleanup.
- No functional changes.
drivers/soc/rockchip/dtpm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/soc/rockchip/dtpm.c +3 -3
@@ -11,7 +11,7 @@#include <linux/of.h> #include <linux/platform_device.h> -static struct dtpm_node rk3399_virtual = { +static struct dtpm_node rockchip_virtual = { .type =DTPM_NODE_VIRTUAL , };
@@ -33,12 +33,12 @@ static struct dtpm_node rk3399_gpu = {static struct powercap_node rk3399_nodes[]__initdata = { [0 ] = { .name ="rk3399" , - .data = &rk3399_virtual, + .data = &rockchip_virtual, }, [1 ] = { .name ="package" , .parent = &rk3399_nodes[0 ], - .data = &rk3399_virtual, + .data = &rockchip_virtual, }, [2 ] = { .name ="cpu0-cpufreq" ,
Signature · 1 line
2.43.0
Re: [PATCH v4 5/6] powercap: dtpm: Rename Rockchip virtual node descriptor
Quoted 38 lines · Daniel Lezcano
The virtual DTPM node descriptor is named after the RK3399 even though it does not contain any SoC-specific information.
Rename it to
rockchip_virtualso it can be reused by other Rockchip SoC hierarchies.No functional change intended.
Signed-off-by Daniel Lezcano <[email protected]>---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.Changes in v3 (since v2):
- Rebase on v7.3-rc7 and refresh context after patch 4 style cleanup.
- No functional changes.drivers/soc/rockchip/dtpm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/soc/rockchip/dtpm.c +1
-2@@ -11,7 +11,7 @@#include <linux/of.h> #include <linux/platform_device.h> -static struct dtpm_node rk3399_virtual = { +static struct dtpm_node rockchip_virtual = { .type =DTPM_NODE_VIRTUAL , };@@ -33,12 +33,12 @@ static struct dtpm_node rk3399_gpu = {static struct powercap_node rk3399_nodes[]__initdata = { [0 ] = { .name ="rk3399" , - .data = &rk3399_virtual,
drivers/soc/rockchip/dtpm.c static struct dtpm_node rk3399_gpu = { · line 44
Quoted 10 lines
drivers/soc/rockchip/dtpm.c +2
-1+ .data = &rockchip_virtual, }, [1] = { .name = "package", .parent = &rk3399_nodes[0], - .data = &rk3399_virtual, + .data = &rockchip_virtual, }, [2] = { .name = "cpu0-cpufreq",
PATCH v4 6/6 powercap: dtpm: Add RK3588 hierarchy
In reply to Daniel LezcanoThe RK3588 has three CPU frequency domains, with CPUs grouped into three cpufreq policies.
Add a DTPM hierarchy for the RK3588 with one CPU node per cpufreq policy, represented by cpu@0, cpu@400 and cpu@600. Group the three CPU domains under a virtual package node attached to the RK3588 root node.
Add the RK3588 compatible to the Rockchip DTPM match table to select the new hierarchy.
---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.
Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Fix __initdata placement and the RK3588 match-entry indentation.
- No functional changes.
drivers/soc/rockchip/dtpm.c | 48 +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
drivers/soc/rockchip/dtpm.c +48 -0
@@ -62,8 +62,56 @@ static struct powercap_hierarchy rk3399_hierarchy __initdata = { .nr_nodes = ARRAY_SIZE(rk3399_nodes), }; +static struct dtpm_node rk3588_cpu0 = { + .type =DTPM_NODE_DT , + .path ="/cpus/cpu@0" , +}; + +static struct dtpm_node rk3588_cpu4 = { + .type =DTPM_NODE_DT , + .path ="/cpus/cpu@400" , +}; + +static struct dtpm_node rk3588_cpu6 = { + .type =DTPM_NODE_DT , + .path ="/cpus/cpu@600" , +}; + +static struct powercap_node rk3588_nodes[]__initdata = { + [0 ] = { + .name ="rk3588" , + .data = &rockchip_virtual, + }, + [1 ] = { + .name ="package" , + .parent = &rk3588_nodes[0 ], + .data = &rockchip_virtual, + }, + [2 ] = { + .name ="cpu0-cpufreq" , + .parent = &rk3588_nodes[1 ], + .data = &rk3588_cpu0, + }, + [3 ] = { + .name ="cpu4-cpufreq" , + .parent = &rk3588_nodes[1 ], + .data = &rk3588_cpu4, + }, + [4 ] = { + .name ="cpu6-cpufreq" , + .parent = &rk3588_nodes[1 ], + .data = &rk3588_cpu6, + }, +}; + +static struct powercap_hierarchy rk3588_hierarchy__initdata = { + .nodes = rk3588_nodes, + .nr_nodes =ARRAY_SIZE (rk3588_nodes), +}; +static struct of_device_id __initdata rockchip_dtpm_match_table[] = { { .compatible ="rockchip,rk3399" , .data = &rk3399_hierarchy }, + { .compatible ="rockchip,rk3588" , .data = &rk3588_hierarchy }, {}, };
Signature · 1 line
2.43.0
Re: [PATCH v4 6/6] powercap: dtpm: Add RK3588 hierarchy
Quoted 74 lines · Daniel Lezcano
The RK3588 has three CPU frequency domains, with CPUs grouped into three cpufreq policies.
Add a DTPM hierarchy for the RK3588 with one CPU node per cpufreq policy, represented by cpu@0, cpu@400 and cpu@600. Group the three CPU domains under a virtual package node attached to the RK3588 root node.
Add the RK3588 compatible to the Rockchip DTPM match table to select the new hierarchy.
Signed-off-by Daniel Lezcano <[email protected]>---
Changes in v4 (since v3):
- Rebase on v7.3; no code or commit-description changes.Changes in v3 (since v2):
- Rebase on v7.3-rc7.
- Fix__initdataplacement and the RK3588 match-entry indentation.
- No functional changes.drivers/soc/rockchip/dtpm.c | 48 +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)drivers/soc/rockchip/dtpm.c +43
-0@@ -62,8 +62,56 @@ static struct powercap_hierarchy rk3399_hierarchy __initdata = { .nr_nodes = ARRAY_SIZE(rk3399_nodes), }; +static struct dtpm_node rk3588_cpu0 = { + .type =DTPM_NODE_DT , + .path ="/cpus/cpu@0" , +}; + +static struct dtpm_node rk3588_cpu4 = { + .type =DTPM_NODE_DT , + .path ="/cpus/cpu@400" , +}; + +static struct dtpm_node rk3588_cpu6 = { + .type =DTPM_NODE_DT , + .path ="/cpus/cpu@600" , +}; + +static struct powercap_node rk3588_nodes[]__initdata = { + [0 ] = { + .name ="rk3588" , + .data = &rockchip_virtual, + }, + [1 ] = { + .name ="package" , + .parent = &rk3588_nodes[0 ], + .data = &rockchip_virtual, + }, + [2 ] = { + .name ="cpu0-cpufreq" , + .parent = &rk3588_nodes[1 ], + .data = &rk3588_cpu0, + }, + [3 ] = { + .name ="cpu4-cpufreq" , + .parent = &rk3588_nodes[1 ], + .data = &rk3588_cpu4, + }, + [4 ] = { + .name ="cpu6-cpufreq" , + .parent = &rk3588_nodes[1 ], + .data = &rk3588_cpu6, + }, +}; + +static struct powercap_hierarchy rk3588_hierarchy__initdata = {
drivers/soc/rockchip/dtpm.c static struct powercap_hierarchy rk3399_hierarchy __initdata = { · line 117
Quoted 10 lines
drivers/soc/rockchip/dtpm.c +5
-0+ .nodes = rk3588_nodes, + .nr_nodes = ARRAY_SIZE(rk3588_nodes), +}; +static struct of_device_id __initdata rockchip_dtpm_match_table[] = { { .compatible ="rockchip,rk3399" , .data = &rk3399_hierarchy }, + { .compatible ="rockchip,rk3588" , .data = &rk3588_hierarchy }, {}, };
Hi,
On 10/29/26 17:56, Daniel Lezcano wrote:
[ ... ]
Quoted 7 lines
Changes in v4 (since v3):
- Rebase on v7.3.
- Address the selftest issues reported by Sashiko:
- Add a kselftest config fragment requestingCONFIG_POWERCAP,CONFIG_MODULESandCONFIG_MODULE_UNLOAD, guard the module build withCONFIG_POWERCAP, and skip building when prerequisites or Module.symvers are missing instead of failing at modpost.
Quoted 8 lines
- Resolve the kernel output directory and pass absolute O=,
KBUILD_OUTPUTand module-output paths toKbuildso cleanup works when a relative O= is inherited from the parent selftest make invocation.
- Keep the statically initialized mutex in permanent module data so its lockdep key does not point into freed module init memory. Retain a separate init-only description for the hierarchy duplication test.
- ReturnKSFT_SKIPwhen powercap sysfs support or the test module is unavailable, while retaining failures for actual module-load errors.
Just a heads up to say the series passed sashiko
Thanks
-- Daniel