[PATCH v2 0/6] powercap: Add generic hierarchy support and convert DTPM

[PATCH v2 0/6] powercap: Add generic hierarchy support and convert DTPM

[Daniel Lezcano]

Powercap controllers frequently expose a hierarchy of power zones, for
example packages, clusters and CPUs. Until now, each driver had to
implement its own hierarchy traversal, parent lookup, error handling and
teardown logic.

This series introduces a generic hierarchy description and helper
functions allowing a powercap controller to instantiate and destroy an
entire hierarchy from a static description. The powercap core takes care
of duplicating the hierarchy description, rebasing parent pointers,
creating zones in dependency order, and performing the appropriate
rollback and cleanup on errors.

The API was primarily motivated by the recently posted SPEL series and
also provides a migration path for DTPM to use the same generic
infrastructure instead of maintaining its own hierarchy handling.

Link: SPEL cover letter https://lore.kernel.org/lkml/20260702-qcom_spel_driver_upstream-v3-0-434d50f0c5b0@oss.qualcomm.com/

A kselftest is added to exercise the new API. It creates a synthetic
hierarchy, validates the resulting sysfs hierarchy and attributes, and
checks that all resources are correctly released when the hierarchy is
destroyed.

DTPM currently implements its own hierarchy creation logic to walk a
platform-provided description, create the corresponding DTPM zones and
tear them down. With the generic powercap hierarchy infrastructure in
place, this logic can instead be shared at the powercap core level.

The series therefore converts DTPM to use the generic powercap hierarchy
helpers. The platform hierarchy is duplicated before use, while
powercap_hierarchy_create() and powercap_hierarchy_destroy() handle its
lifetime and traversal. DTPM only provides the callbacks responsible for
creating and destroying the DTPM instance associated with each powercap
node. The CPU and devfreq backends are adjusted accordingly.

As part of this conversion, the existing RK3399 description is
simplified. The CPU backend creates one DTPM zone per cpufreq policy, so
describing every CPU belonging to the same policy results in duplicate
zones. The RK3399 hierarchy is therefore reduced to one CPU node per
cpufreq policy.

The Rockchip virtual node descriptor is also made SoC-independent so it
can be reused by other Rockchip platforms.

Finally, an RK3588 hierarchy is added. The RK3588 exposes three cpufreq
policies, represented by cpu@0, cpu@400 and cpu@600. These three CPU
domains are grouped below a virtual package node:

rk3588
  `-- package
    |-- cpu0-cpufreq
    |-- cpu4-cpufreq
    `-- cpu6-cpufreq

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                       | 165 ++++++------
 drivers/powercap/dtpm_cpu.c                   |  22 +-
 drivers/powercap/dtpm_devfreq.c               |  22 +-
 drivers/powercap/powercap_sys.c               | 148 +++++++++++
 drivers/soc/rockchip/dtpm.c                   | 127 ++++++---
 include/linux/dtpm.h                          |   5 +-
 include/linux/powercap.h                      | 178 +++++++++++++
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/powercap/Kbuild       |   3 +
 tools/testing/selftests/powercap/Makefile     |  17 ++
 .../selftests/powercap/powercap_hierarchy.c   | 246 ++++++++++++++++++
 .../selftests/powercap/powercap_hierarchy.sh  | 118 +++++++++
 12 files changed, 906 insertions(+), 146 deletions(-)
 create mode 100644 tools/testing/selftests/powercap/Kbuild
 create mode 100644 tools/testing/selftests/powercap/Makefile
 create mode 100644 tools/testing/selftests/powercap/powercap_hierarchy.c
 create mode 100755 tools/testing/selftests/powercap/powercap_hierarchy.sh

-- 
2.43.0

[PATCH v2 1/6] powercap: Add generic zone hierarchy creation helpers

[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_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 = {
          .nodes = nodes,
          .nr_nodes = ARRAY_SIZE(nodes),
  };

creates the following hierarchy:

  package
  |-- cpu
  `-- gpu

Add powercap_hierarchy_dup() to create a runtime copy of a hierarchy
description. Rebase the parent pointers so that the copy does not keep
references to the original array, which may be stored in init memory.

Add powercap_hierarchy_create() to walk the description in order
and create each zone through a controller-provided callback. The parent
zone is passed to the callback, keeping the node creation operation
specific to the controller.

The backend is responsible for allocating and registering each powercap
zone from the creation callback. The powercap_zone object is expected to
be embedded in a backend-specific structure, allowing the backend
callbacks to retrieve their private data later using container_of().

For example:

  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);

Its allocation, private state and lifetime remain under the control of
the backend.

The creation callback therefore returns the same powercap_zone pointer
that was passed to powercap_register_zone(). The hierarchy helper stores
this pointer to provide it as the parent of subsequent nodes and to pass
it back to the backend during hierarchy destruction.

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.

Cc: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
Link: https://patch.msgid.link/20260806110159.69690-2-daniel.lezcano@oss.qualcomm.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 drivers/powercap/powercap_sys.c | 148 ++++++++++++++++++++++++++
 include/linux/powercap.h        | 178 ++++++++++++++++++++++++++++++++
 2 files changed, 326 insertions(+)

diff --git a/drivers/powercap/powercap_sys.c b/drivers/powercap/powercap_sys.c
index 9197fa20d93f..fefb36c5bcd2 100644
--- a/drivers/powercap/powercap_sys.c
+++ b/drivers/powercap/powercap_sys.c
@@ -667,6 +667,154 @@ 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(&copy->lock);
+
+	for (i = 0; i < copy->nr_nodes; i++) {
+		const struct powercap_node *parent = hierarchy->nodes[i].parent;
+		ptrdiff_t index;
+
+		copy->nodes[i].pcz = 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 = &copy->nodes[index];
+	}
+
+	return copy;
+
+invalid:
+	mutex_destroy(&copy->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)
+{
+	size_t i;
+
+	for (i = hierarchy->nr_nodes; i-- > 0;) {
+		if (!hierarchy->nodes[i].pcz)
+			continue;
+
+		powercap_node_destroy(pct, hierarchy->nodes[i].pcz,
+				      hierarchy->nodes[i].data);
+
+		hierarchy->nodes[i].pcz = 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;
+
+	guard(mutex)(&hierarchy->lock);
+
+	__powercap_hierarchy_destroy(pct, hierarchy, powercap_node_destroy);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(powercap_hierarchy_destroy);
+
+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;
+
+	guard(mutex)(&hierarchy->lock);
+
+	for (i = 0; i < hierarchy->nr_nodes; i++) {
+		struct powercap_zone *parent = NULL;
+
+		if (hierarchy->nodes[i].parent) {
+			parent = hierarchy->nodes[i].parent->pcz;
+			if (!parent) {
+				ret = -EINVAL;
+				goto rollback;
+			}
+		}
+
+		pcz = powercap_node_create(pct, hierarchy->nodes[i].name,
+					   hierarchy->nodes[i].data, parent);
+		if (IS_ERR_OR_NULL(pcz)) {
+			ret = pcz ? PTR_ERR(pcz) : -EINVAL;
+			goto rollback;
+		}
+
+		hierarchy->nodes[i].pcz = pcz;
+	}
+
+	return 0;
+
+rollback:
+	__powercap_hierarchy_destroy(pct, hierarchy, powercap_node_destroy);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(powercap_hierarchy_create);
+
 static int __init powercap_init(void)
 {
 	int result;
diff --git a/include/linux/powercap.h b/include/linux/powercap.h
index 603419db924c..939cd58dff4f 100644
--- a/include/linux/powercap.h
+++ b/include/linux/powercap.h
@@ -9,6 +9,7 @@
 
 #include <linux/device.h>
 #include <linux/idr.h>
+#include <linux/mutex.h>
 
 /*
  * A power cap class device can contain multiple powercap control_types.
@@ -309,4 +310,181 @@ struct powercap_zone *powercap_register_zone(
 int powercap_unregister_zone(struct powercap_control_type *control_type,
 				struct powercap_zone *power_zone);
 
+/**
+ * struct powercap_node - Description of a node in a powercap hierarchy
+ * @name: Name of the powercap zone.
+ * @parent: Parent node, or NULL if the node is a hierarchy root.
+ * @pcz: Powercap zone created for this node. This field is managed by the
+ *	 hierarchy creation and destruction helpers.
+ * @data: Private data passed unchanged to the creation and destruction
+ *	  callbacks.
+ *
+ * This structure describes one node of a powercap hierarchy. The backend
+ * supplies an array of nodes through &struct powercap_hierarchy.
+ *
+ * Nodes must be ordered so that a parent appears before all its children.
+ *
+ * The @name and @data objects must remain valid until the duplicated hierarchy
+ * has been freed. The @parent pointer is rebased by powercap_hierarchy_dup(),
+ * allowing the original node array to be released after duplication.
+ *
+ * The @pcz field is runtime state managed by powercap_hierarchy_create() and
+ * powercap_hierarchy_destroy(). It is cleared when a node is destroyed.
+ */
+struct powercap_node {
+	const char *name;
+	struct powercap_node *parent;
+	struct powercap_zone *pcz;
+	void *data;
+};
+
+/**
+ * struct powercap_hierarchy - Powercap zone hierarchy
+ * @nodes: Array describing the hierarchy nodes.
+ * @nr_nodes: Number of entries in @nodes.
+ * @lock: Lock serializing hierarchy creation and destruction.
+ *
+ * A backend may place the initial description in init memory and duplicate it
+ * with powercap_hierarchy_dup() before the init sections are released. The
+ * duplicated hierarchy owns the @nodes array, but not the objects referenced
+ * by &struct powercap_node.name and &struct powercap_node.data.
+ */
+struct powercap_hierarchy {
+	struct powercap_node *nodes;
+	size_t nr_nodes;
+	struct mutex lock;
+};
+
+/**
+ * powercap_hierarchy_dup - Duplicate a powercap hierarchy description
+ * @hierarchy: Hierarchy description to duplicate.
+ *
+ * Allocate a runtime hierarchy and copy the nodes from @hierarchy. Parent
+ * pointers are rebased to the duplicated node array and the runtime pcz fields
+ * are initialized to NULL.
+ *
+ * Every parent must belong to the source node array and precede its children.
+ * The node names and private data are not duplicated and must remain valid
+ * until powercap_hierarchy_free() is called.
+ *
+ * Context: Process context. May sleep.
+ *
+ * Return: A pointer to the duplicated hierarchy on success, or an ERR_PTR()
+ * encoded error otherwise.
+ */
+struct powercap_hierarchy *powercap_hierarchy_dup(const struct powercap_hierarchy *hierarchy);
+
+/**
+ * powercap_hierarchy_free - Free a duplicated powercap hierarchy
+ * @hierarchy: Hierarchy to free, or NULL.
+ *
+ * Free the node array and hierarchy allocated by powercap_hierarchy_dup(). All
+ * registered zones must have been destroyed before calling this function.
+ */
+void powercap_hierarchy_free(struct powercap_hierarchy *hierarchy);
+
+/**
+ * typedef powercap_node_create_t - Create a powercap hierarchy node
+ * @pct: Powercap control type owning the hierarchy.
+ * @name: Name of the powercap zone to create.
+ * @data: Private data associated with the hierarchy node.
+ * @parent: Parent powercap zone, or NULL for a root node.
+ *
+ * Callback invoked by powercap_hierarchy_create() for each node in the
+ * hierarchy. The callback must create and register a powercap zone below
+ * @parent. The backend is expected to embed struct powercap_zone in its own
+ * object, pass the address of that member to powercap_register_zone(), and
+ * return the same address from this callback. This lets the backend recover
+ * its object with container_of() from subsequent powercap callbacks.
+ *
+ * Context: Called with the powercap hierarchy mutex held. The callback may
+ * sleep, but must not call powercap_hierarchy_create() or
+ * powercap_hierarchy_destroy() for the same hierarchy.
+ *
+ * Return: A valid pointer to the created powercap zone on success, or an
+ * ERR_PTR() encoded error on failure.
+ */
+typedef struct powercap_zone *(*powercap_node_create_t)(struct powercap_control_type *pct,
+							const char *name, void *data,
+							struct powercap_zone *parent);
+/**
+ * typedef powercap_node_destroy_t - Destroy a powercap hierarchy node
+ * @pct: Powercap control type owning the hierarchy.
+ * @zone: Powercap zone to destroy.
+ * @data: Private data associated with the hierarchy node.
+ *
+ * Callback invoked when a hierarchy is destroyed or when its creation must
+ * be rolled back. The callback must unregister the powercap zone represented
+ * by @zone. The backend remains responsible for the lifetime of the enclosing
+ * object, including releasing it from the powercap zone release callback when
+ * necessary.
+ *
+ * Nodes are passed to this callback in reverse creation order, ensuring that
+ * all children are destroyed before their parent.
+ *
+ * Context: Called with the powercap hierarchy mutex held. The callback may
+ * sleep, but must not call powercap_hierarchy_create() or
+ * powercap_hierarchy_destroy() for the same hierarchy.
+ */
+typedef void (*powercap_node_destroy_t)(struct powercap_control_type *pct,
+					struct powercap_zone *zone,
+					void *data);
+
+/**
+ * powercap_hierarchy_destroy - Destroy a powercap hierarchy
+ * @pct: Powercap control type owning the hierarchy.
+ * @hierarchy: Hierarchy to destroy.
+ * @powercap_node_destroy: Callback used to destroy each powercap zone.
+ *
+ * Destroy all powercap zones previously created for @hierarchy. Nodes are
+ * destroyed in reverse array order so that children are removed before their
+ * parents.
+ *
+ * Entries whose &struct powercap_node.pcz field is NULL are ignored. After
+ * a zone has been destroyed, its pcz field is cleared.
+ *
+ * The caller must ensure that no users of the hierarchy remain when this
+ * function is called.
+ *
+ * Context: Process context. May sleep.
+ *
+ * Return: 0 on success or -EINVAL if an argument is invalid.
+ */
+int powercap_hierarchy_destroy(struct powercap_control_type *pct,
+			       struct powercap_hierarchy *hierarchy,
+			       powercap_node_destroy_t powercap_node_destroy);
+
+/**
+ * powercap_hierarchy_create - Create a powercap hierarchy
+ * @pct: Powercap control type that will own the hierarchy.
+ * @hierarchy: Hierarchy to create.
+ * @powercap_node_create: Callback used to create each powercap zone.
+ * @powercap_node_destroy: Callback used to roll back an incomplete hierarchy.
+ *
+ * Create the powercap zones described by @hierarchy in array order. For each
+ * entry, @powercap_node_create is called with the powercap zone stored in its
+ * parent entry. Root nodes are created with a NULL parent.
+ *
+ * Each parent entry must precede all its children in the node array. All pcz
+ * fields must be NULL when this function is called.
+ *
+ * If a node cannot be created, all zones created by this invocation are
+ * destroyed in reverse order by calling @powercap_node_destroy. Therefore,
+ * @powercap_node_destroy must be provided even when the caller does not
+ * expect to destroy the hierarchy explicitly.
+ *
+ * The hierarchy and the objects referenced by its name and data fields must
+ * remain valid until powercap_hierarchy_destroy() has completed.
+ *
+ * Context: Process context. May sleep.
+ *
+ * Return: 0 on success, -EINVAL if an argument or hierarchy entry is invalid,
+ * -EBUSY if the hierarchy already contains a created zone, or the error
+ * returned by @powercap_node_create.
+ */
+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);
+
 #endif
-- 
2.43.0

[PATCH v2 2/6] selftests/powercap: Add powercap hierarchy creation API tests

[Daniel Lezcano]

The 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 verifies that the
expected sysfs hierarchy and attributes are created. It also validates
that the hierarchy is correctly removed when the module is unloaded.

The test consists of:

* a kernel module implementing a synthetic powercap hierarchy and
  dummy callbacks;
* a userspace kselftest script that loads the module, validates the
  exported sysfs hierarchy and attribute values, unloads the module
  and verifies that all objects have been removed.

This provides a regression test for the powercap hierarchy helpers and
their integration with the powercap core.

Cc: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
Link: https://patch.msgid.link/20260806110159.69690-3-daniel.lezcano@oss.qualcomm.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/powercap/Kbuild       |   3 +
 tools/testing/selftests/powercap/Makefile     |  17 ++
 .../selftests/powercap/powercap_hierarchy.c   | 246 ++++++++++++++++++
 .../selftests/powercap/powercap_hierarchy.sh  | 118 +++++++++
 5 files changed, 385 insertions(+)
 create mode 100644 tools/testing/selftests/powercap/Kbuild
 create mode 100644 tools/testing/selftests/powercap/Makefile
 create mode 100644 tools/testing/selftests/powercap/powercap_hierarchy.c
 create mode 100755 tools/testing/selftests/powercap/powercap_hierarchy.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 8d4db2241cc2..2fac671e9145 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -93,6 +93,7 @@ TARGETS += pidfd
 TARGETS += pid_namespace
 TARGETS += pipe
 TARGETS += power_supply
+TARGETS += powercap
 TARGETS += powerpc
 TARGETS += prctl
 TARGETS += proc
diff --git a/tools/testing/selftests/powercap/Kbuild b/tools/testing/selftests/powercap/Kbuild
new file mode 100644
index 000000000000..baa0d0ba9593
--- /dev/null
+++ b/tools/testing/selftests/powercap/Kbuild
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-m += powercap_hierarchy.o
\ No newline at end of file
diff --git a/tools/testing/selftests/powercap/Makefile b/tools/testing/selftests/powercap/Makefile
new file mode 100644
index 000000000000..210d6e6c5577
--- /dev/null
+++ b/tools/testing/selftests/powercap/Makefile
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# User-space test script
+TEST_PROGS := powercap_hierarchy.sh
+
+# Kernel module built as part of the test
+TEST_FILES := powercap_hierarchy.ko
+
+KDIR ?= $(if $(O),$(O),$(realpath ../../../..))
+
+all:
+	$(MAKE) -C $(KDIR) M=$(CURDIR) modules
+
+clean:
+	$(MAKE) -C $(KDIR) M=$(CURDIR) clean
+
+include ../lib.mk
diff --git a/tools/testing/selftests/powercap/powercap_hierarchy.c b/tools/testing/selftests/powercap/powercap_hierarchy.c
new file mode 100644
index 000000000000..ab3f1220f909
--- /dev/null
+++ b/tools/testing/selftests/powercap/powercap_hierarchy.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
+ *
+ * Powercap hierarchy description test module
+ */
+#include <linux/powercap.h>
+
+struct pch_test_data {
+	int value;
+};
+
+static struct powercap_node __initdata pch_test_nodes[] = {
+	[0] = { .name = "package",
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[1] = { .name = "cluster0", .parent = &pch_test_nodes[0],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[2] = { .name = "cluster1", .parent = &pch_test_nodes[0],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[3] = { .name = "cluster2", .parent = &pch_test_nodes[0],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[4] = { .name = "cpu0", .parent = &pch_test_nodes[1],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[5] = { .name = "cpu1", .parent = &pch_test_nodes[1],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[6] = { .name = "cpu2", .parent = &pch_test_nodes[1],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[7] = { .name = "cpu3", .parent = &pch_test_nodes[1],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[8] = { .name = "cpu4", .parent = &pch_test_nodes[2],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[9] = { .name = "cpu5", .parent = &pch_test_nodes[2],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[10] = { .name = "cpu6", .parent = &pch_test_nodes[2],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[11] = { .name = "cpu7", .parent = &pch_test_nodes[2],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[12] = { .name = "cpu8", .parent = &pch_test_nodes[3],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[13] = { .name = "cpu9", .parent = &pch_test_nodes[3],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[14] = { .name = "cpu10", .parent = &pch_test_nodes[3],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+	[15] = { .name = "cpu11", .parent = &pch_test_nodes[3],
+		.data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
+};
+
+static struct powercap_hierarchy __initdata pch_test_hierarchy = {
+	.nodes = pch_test_nodes,
+	.nr_nodes = ARRAY_SIZE(pch_test_nodes),
+};
+
+static struct powercap_hierarchy *hierarchy;
+
+static struct powercap_control_type *pct;
+
+struct pch_test {
+	struct powercap_zone zone;
+};
+
+static struct pch_test *to_pch_test(struct powercap_zone *pcz)
+{
+	return container_of(pcz, struct pch_test, zone);
+}
+
+static int pch_test_get_max_power_range_uw(struct powercap_zone *pcz,
+					   u64 *power_uw)
+{
+	*power_uw = 0xBADC0FFEE;
+	return 0;
+}
+
+static int pch_test_get_power_uw(struct powercap_zone *pcz,
+				 u64 *power_uw)
+{
+	*power_uw = 0xC0FFEE;
+	return 0;
+}
+
+static int pch_test_release(struct powercap_zone *pcz)
+{
+	kfree(to_pch_test(pcz));
+	return 0;
+}
+
+static const struct powercap_zone_ops pch_test_ops = {
+	.get_max_power_range_uw = pch_test_get_max_power_range_uw,
+	.get_power_uw = pch_test_get_power_uw,
+	.release = pch_test_release,
+};
+
+static int pch_test_set_power_limit_uw(struct powercap_zone *pcz,
+				       int cid, u64 power_uw)
+{
+	return 0;
+}
+
+static int pch_test_get_power_limit_uw(struct powercap_zone *pcz,
+				       int cid, u64 *power_uw)
+{
+	*power_uw = 0xDEADC0DE;
+	return 0;
+}
+
+static int pch_test_set_time_window_us(struct powercap_zone *pcz,
+				       int cid, u64 power_uw)
+{
+	return 0;
+}
+
+static int pch_test_get_time_window_us(struct powercap_zone *pcz,
+				       int cid, u64 *power_uw)
+{
+	*power_uw = 0xDEADC0DE;
+	return 0;
+}
+
+static int pch_test_get_max_power_uw(struct powercap_zone *pcz,
+				     int cid, u64 *power_uw)
+{
+	*power_uw = 0xDEADC0DE;
+	return 0;
+}
+
+static const char *pch_test_get_name(struct powercap_zone *pcz, int cid)
+{
+	return "my constraint name";
+}
+
+static const struct powercap_zone_constraint_ops pch_test_constraint_ops = {
+	.set_power_limit_uw = pch_test_set_power_limit_uw,
+	.get_power_limit_uw = pch_test_get_power_limit_uw,
+	.set_time_window_us = pch_test_set_time_window_us,
+	.get_time_window_us = pch_test_get_time_window_us,
+	.get_max_power_uw = pch_test_get_max_power_uw,
+	.get_name = pch_test_get_name,
+};
+
+static struct powercap_zone *pch_test_create(struct powercap_control_type *pct,
+					     const char *name, void *data,
+					     struct powercap_zone *parent)
+{
+	struct pch_test_data *pcht_data = data;
+	struct pch_test *pcht;
+	struct powercap_zone *pcz;
+
+	if (!pct) {
+		pr_err("Invalid NULL controller type\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!name) {
+		pr_err("Invalid NULL name\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (pcht_data->value != 0xDEADBEEF) {
+		pr_err("Invalid pcht data != 0xDEADBEEF");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pcht = kzalloc_obj(*pcht);
+	if (!pcht)
+		return ERR_PTR(-ENOMEM);
+
+	pcz = powercap_register_zone(&pcht->zone, pct, name, parent,
+				     &pch_test_ops, 1, &pch_test_constraint_ops);
+	if (IS_ERR(pcz)) {
+		pr_err("Failed to register powercap zone '%s': %ld\n",
+		       name, PTR_ERR(pcz));
+	}
+
+	return pcz;
+}
+
+static void pch_test_destroy(struct powercap_control_type *pct,
+			     struct powercap_zone *zone,
+			     void *data)
+{
+	struct pch_test_data *pcht_data = data;
+
+	if (!pct) {
+		pr_err("Invalid NULL controller type\n");
+		return;
+        }
+
+	if (!zone) {
+		pr_err("Invalid NULL zone\n");
+		return;
+        }
+
+	if (pcht_data->value != 0xDEADBEEF) {
+		pr_err("Invalid pcht data != 0xDEADBEEF");
+		return;
+        }
+
+	powercap_unregister_zone(pct, zone);
+}
+
+static int __init pch_test_init(void)
+{
+	int ret;
+
+	hierarchy = powercap_hierarchy_dup(&pch_test_hierarchy);
+	if (IS_ERR(hierarchy)) {
+		ret = PTR_ERR(hierarchy);
+		pr_err("Failed to dup the hierarchy: %d\n", ret);
+		return ret;
+	}
+
+	pct = powercap_register_control_type(NULL, "powercap-test", NULL);
+	if (IS_ERR(pct)) {
+		ret = PTR_ERR(pct);
+		pr_err("Failed to register control type: %d\n", ret);
+		goto out_free_hierarchy;
+	}
+
+	ret = powercap_hierarchy_create(pct, hierarchy, pch_test_create, pch_test_destroy);
+	if (ret) {
+		pr_err("Failed to create the hierarchy: %d\n", ret);
+		goto out_unregister_pct;
+	}
+
+	return 0;
+
+out_unregister_pct:
+	powercap_unregister_control_type(pct);
+out_free_hierarchy:
+	powercap_hierarchy_free(hierarchy);
+	return ret;
+}
+module_init(pch_test_init);
+
+static void __exit pch_test_exit(void)
+{
+	powercap_hierarchy_destroy(pct, hierarchy, pch_test_destroy);
+	powercap_hierarchy_free(hierarchy);
+	powercap_unregister_control_type(pct);
+}
+module_exit(pch_test_exit);
+
+MODULE_DESCRIPTION("Powercap hierarchy test driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@oss.qualcomm.com");
diff --git a/tools/testing/selftests/powercap/powercap_hierarchy.sh b/tools/testing/selftests/powercap/powercap_hierarchy.sh
new file mode 100755
index 000000000000..eb070181ca89
--- /dev/null
+++ b/tools/testing/selftests/powercap/powercap_hierarchy.sh
@@ -0,0 +1,118 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+ksft_pass=0
+ksft_fail=1
+ksft_skip=4
+
+MODULE=powercap_hierarchy
+CONTROL=powercap-test
+SYSFS=/sys/devices/virtual/powercap/$CONTROL
+
+fail()
+{
+	echo "FAIL: $*"
+	exit $ksft_fail
+}
+
+skip()
+{
+	echo "SKIP: $*"
+	exit $ksft_skip
+}
+
+cleanup()
+{
+	if lsmod | grep -q "^${MODULE}\b"; then
+		if ! rmmod "$MODULE"; then
+			echo "WARNING: failed to unload $MODULE"
+		fi
+	fi
+}
+
+trap cleanup EXIT INT TERM
+
+[ "$(id -u)" -eq 0 ] || skip "must be run as root"
+
+insmod ./powercap_hierarchy.ko || fail "failed to load module"
+
+[ -d "$SYSFS" ] || fail "missing $SYSFS"
+
+check_zone()
+{
+	zone=$1
+
+	[ -f "$zone/name" ] || fail "$zone/name missing"
+
+	[ -f "$zone/power_uw" ] || \
+		fail "$zone/power_uw missing"
+
+	[ -f "$zone/max_power_range_uw" ] || \
+		fail "$zone/max_power_range_uw missing"
+
+	power=$(cat "$zone/power_uw")
+	[ "$power" = "12648430" ] || \
+		fail "$zone: unexpected power_uw ($power)"
+
+	max=$(cat "$zone/max_power_range_uw")
+	[ "$max" = "50159747054" ] || \
+		fail "$zone: unexpected max_power_range_uw ($max)"
+
+	constraint="$zone/constraint_0"
+
+	name=$(cat "$constraint""_name")
+	[ "$name" = "my constraint name" ] || \
+		fail "$constraint: bad constraint name"
+
+	pl=$(cat "$constraint""_power_limit_uw")
+	[ "$pl" = "3735929054" ] || \
+		fail "$constraint: bad power limit"
+
+	mp=$(cat "$constraint""_max_power_uw")
+	[ "$mp" = "3735929054" ] || \
+		fail "$constraint: bad max power"
+}
+
+zones=0
+
+find "$SYSFS" -type f -name name | while read namefile
+do
+	zone=$(dirname "$namefile")
+
+	case "$zone" in
+		*/constraint_*)
+			continue
+			;;
+	esac
+
+	check_zone "$zone"
+
+	zones=$((zones + 1))
+done
+
+#
+# Count the number of powercap zones.
+#
+count=$(find "$SYSFS" -type f -name name | \
+	grep -v constraint | wc -l)
+
+[ "$count" -eq 16 ] || \
+	fail "expected 16 zones, got $count"
+
+#
+# Explicitly unload the module.
+#
+cleanup
+
+#
+# Verify that the hierarchy disappeared.
+#
+if [ -d "$SYSFS" ]; then
+	fail "$SYSFS still exists after module removal"
+fi
+
+trap - EXIT INT TERM
+
+echo "PASS"
+
+exit $ksft_pass
-- 
2.43.0

[PATCH v2 3/6] powercap: dtpm: Remove duplicate RK3399 CPU hierarchy nodes

[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. This avoids registering the same DTPM
CPU zone multiple times while preserving the two CPU clusters and the
GPU in the hierarchy.

Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 drivers/soc/rockchip/dtpm.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c
index b36d4f752c30..e6a0c607334b 100644
--- a/drivers/soc/rockchip/dtpm.c
+++ b/drivers/soc/rockchip/dtpm.c
@@ -20,25 +20,13 @@ 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[] = {
-- 
2.43.0

[PATCH v2 4/6] powercap: dtpm: Use generic powercap hierarchy helpers

[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() 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.

Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 drivers/powercap/dtpm.c         | 165 ++++++++++++++------------------
 drivers/powercap/dtpm_cpu.c     |  22 ++---
 drivers/powercap/dtpm_devfreq.c |  22 +++--
 drivers/soc/rockchip/dtpm.c     |  67 +++++++++----
 include/linux/dtpm.h            |   5 +-
 5 files changed, 147 insertions(+), 134 deletions(-)

diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index b7a65e543f19..d719f9595ec5 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -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)
@@ -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,84 @@ 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;
+	if (IS_ERR(dtpm))
+		return ERR_CAST(dtpm);
 
-		dtpm = dtpm_node_callback[hierarchy[i].type](&hierarchy[i], parent);
+	if (!dtpm)
+		return ERR_PTR(-ENODEV);
 
-		/*
-		 * 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 +521,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,9 +553,17 @@ 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++) {
 
@@ -590,6 +580,9 @@ 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:
@@ -601,20 +594,6 @@ int dtpm_create_hierarchy(struct of_device_id *dtpm_match_table)
 }
 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,7 +603,7 @@ 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 +614,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;
diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c
index 21355db6419d..c8e02eda701c 100644
--- a/drivers/powercap/dtpm_cpu.c
+++ b/drivers/powercap/dtpm_cpu.c
@@ -189,22 +189,21 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu)
 	return 0;
 }
 
-static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
+static struct dtpm *__dtpm_cpu_setup(int cpu, struct dtpm *parent, const char *name)
 {
 	struct dtpm_cpu *dtpm_cpu;
 	struct cpufreq_policy *policy;
 	struct em_perf_state *table;
 	struct em_perf_domain *pd;
-	char name[CPUFREQ_NAME_LEN];
-	int ret = -ENOMEM;
+	int ret;
 
 	dtpm_cpu = per_cpu(dtpm_per_cpu, cpu);
 	if (dtpm_cpu)
-		return 0;
+		return NULL;
 
 	policy = cpufreq_cpu_get(cpu);
 	if (!policy)
-		return 0;
+		return NULL;
 
 	pd = em_cpu_get(cpu);
 	if (!pd || em_is_artificial(pd)) {
@@ -224,8 +223,6 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
 	for_each_cpu(cpu, policy->related_cpus)
 		per_cpu(dtpm_per_cpu, cpu) = dtpm_cpu;
 
-	snprintf(name, sizeof(name), "cpu%d-cpufreq", dtpm_cpu->cpu);
-
 	ret = dtpm_register(name, &dtpm_cpu->dtpm, parent);
 	if (ret)
 		goto out_kfree_dtpm_cpu;
@@ -240,7 +237,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
 		goto out_dtpm_unregister;
 
 	cpufreq_cpu_put(policy);
-	return 0;
+	return &dtpm_cpu->dtpm;
 
 out_dtpm_unregister:
 	dtpm_unregister(&dtpm_cpu->dtpm);
@@ -253,18 +250,19 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
 
 release_policy:
 	cpufreq_cpu_put(policy);
-	return ret;
+	return ERR_PTR(ret);
 }
 
-static int dtpm_cpu_setup(struct dtpm *dtpm, struct device_node *np)
+static struct dtpm *dtpm_cpu_setup(struct dtpm *dtpm, struct device_node *np,
+				   const char *name)
 {
 	int cpu;
 
 	cpu = of_cpu_node_to_id(np);
 	if (cpu < 0)
-		return 0;
+		return NULL;
 
-	return __dtpm_cpu_setup(cpu, dtpm);
+	return __dtpm_cpu_setup(cpu, dtpm, name);
 }
 
 static int dtpm_cpu_init(void)
diff --git a/drivers/powercap/dtpm_devfreq.c b/drivers/powercap/dtpm_devfreq.c
index fa71285dee60..a783601fe74e 100644
--- a/drivers/powercap/dtpm_devfreq.c
+++ b/drivers/powercap/dtpm_devfreq.c
@@ -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 = {
diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c
index e6a0c607334b..451417ae5715 100644
--- a/drivers/soc/rockchip/dtpm.c
+++ b/drivers/soc/rockchip/dtpm.c
@@ -11,26 +11,59 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
-static struct dtpm_node __initdata rk3399_hierarchy[] = {
-	[0] = { .name = "rk3399",
-		.type = DTPM_NODE_VIRTUAL },
-	[1] = { .name = "package",
-		.type = DTPM_NODE_VIRTUAL,
-		.parent = &rk3399_hierarchy[0] },
-	[2] = { .name = "/cpus/cpu@0",
-		.type = DTPM_NODE_DT,
-		.parent = &rk3399_hierarchy[1] },
-	[3] = { .name = "/cpus/cpu@100",
-		.type = DTPM_NODE_DT,
-		.parent = &rk3399_hierarchy[1] },
-	[4] = { .name = "/gpu@ff9a0000",
-		.type = DTPM_NODE_DT,
-		.parent = &rk3399_hierarchy[1] },
-	[5] = { /* sentinel */ }
+static struct dtpm_node rk3399_virtual = {
+	.type = DTPM_NODE_VIRTUAL,
+};
+
+static struct dtpm_node rk3399_cpu0 = {
+	.type = DTPM_NODE_DT,
+	.path = "/cpus/cpu@0",
+};
+
+static struct dtpm_node rk3399_cpu4 = {
+	.type = DTPM_NODE_DT,
+	.path = "/cpus/cpu@100",
+};
+
+static struct dtpm_node rk3399_gpu = {
+	.type = DTPM_NODE_DT,
+	.path = "/gpu@ff9a0000",
+};
+
+static struct powercap_node __initdata rk3399_nodes[] = {
+	[0] = {
+		.name = "rk3399",
+		.data = &rk3399_virtual,
+	},
+	[1] = {
+		.name = "package",
+		.parent = &rk3399_nodes[0],
+		.data = &rk3399_virtual,
+	},
+	[2] = {
+		.name = "cpu0-cpufreq",
+		.parent = &rk3399_nodes[1],
+		.data = &rk3399_cpu0,
+	},
+	[3] = {
+		.name = "cpu4-cpufreq",
+		.parent = &rk3399_nodes[1],
+		.data = &rk3399_cpu4,
+	},
+	[4] = {
+		.name = "ff9a0000.gpu",
+		.parent = &rk3399_nodes[1],
+		.data = &rk3399_gpu,
+	},
+};
+
+static struct powercap_hierarchy __initdata rk3399_hierarchy = {
+	.nodes = rk3399_nodes,
+	.nr_nodes = ARRAY_SIZE(rk3399_nodes),
 };
 
 static struct of_device_id __initdata rockchip_dtpm_match_table[] = {
-        { .compatible = "rockchip,rk3399", .data = rk3399_hierarchy },
+        { .compatible = "rockchip,rk3399", .data = &rk3399_hierarchy },
         {},
 };
 
diff --git a/include/linux/dtpm.h b/include/linux/dtpm.h
index a4a13514b730..c0ece6a4d1fe 100644
--- a/include/linux/dtpm.h
+++ b/include/linux/dtpm.h
@@ -38,7 +38,7 @@ 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 *, struct device_node *, const char *);
 };
 
 enum DTPM_NODE_TYPE {
@@ -48,8 +48,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)
-- 
2.43.0

[PATCH v2 5/6] powercap: dtpm: Rename Rockchip virtual node descriptor

[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_virtual so it can be reused by other Rockchip SoC
hierarchies.

No functional change intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 drivers/soc/rockchip/dtpm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c
index 451417ae5715..5f7f238a15fe 100644
--- a/drivers/soc/rockchip/dtpm.c
+++ b/drivers/soc/rockchip/dtpm.c
@@ -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 __initdata rk3399_nodes[] = {
 	[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",
-- 
2.43.0

[PATCH v2 6/6] powercap: dtpm: Add RK3588 hierarchy

[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 <daniel.lezcano@oss.qualcomm.com>
---
 drivers/soc/rockchip/dtpm.c | 48 +++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c
index 5f7f238a15fe..3d9367d05803 100644
--- a/drivers/soc/rockchip/dtpm.c
+++ b/drivers/soc/rockchip/dtpm.c
@@ -62,8 +62,56 @@ static struct powercap_hierarchy __initdata rk3399_hierarchy = {
 	.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 __initdata rk3588_nodes[] = {
+	[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 __initdata rk3588_hierarchy = {
+	.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 },
         {},
 };
 
-- 
2.43.0

Re: [PATCH v2 0/6] powercap: Add generic hierarchy support and convert DTPM

[Daniel Lezcano]

Hi Rafael,

just a gentle ping on this series ;)

Thanks


On 9/23/26 16:34, Daniel Lezcano wrote:
> Powercap controllers frequently expose a hierarchy of power zones, for
> example packages, clusters and CPUs. Until now, each driver had to
> implement its own hierarchy traversal, parent lookup, error handling and
> teardown logic.
> 
> This series introduces a generic hierarchy description and helper
> functions allowing a powercap controller to instantiate and destroy an
> entire hierarchy from a static description. The powercap core takes care
> of duplicating the hierarchy description, rebasing parent pointers,
> creating zones in dependency order, and performing the appropriate
> rollback and cleanup on errors.
> 
> The API was primarily motivated by the recently posted SPEL series and
> also provides a migration path for DTPM to use the same generic
> infrastructure instead of maintaining its own hierarchy handling.
> 
> Link: SPEL cover letter https://lore.kernel.org/lkml/20260702-qcom_spel_driver_upstream-v3-0-434d50f0c5b0@oss.qualcomm.com/
> 
> A kselftest is added to exercise the new API. It creates a synthetic
> hierarchy, validates the resulting sysfs hierarchy and attributes, and
> checks that all resources are correctly released when the hierarchy is
> destroyed.
> 
> DTPM currently implements its own hierarchy creation logic to walk a
> platform-provided description, create the corresponding DTPM zones and
> tear them down. With the generic powercap hierarchy infrastructure in
> place, this logic can instead be shared at the powercap core level.
> 
> The series therefore converts DTPM to use the generic powercap hierarchy
> helpers. The platform hierarchy is duplicated before use, while
> powercap_hierarchy_create() and powercap_hierarchy_destroy() handle its
> lifetime and traversal. DTPM only provides the callbacks responsible for
> creating and destroying the DTPM instance associated with each powercap
> node. The CPU and devfreq backends are adjusted accordingly.
> 
> As part of this conversion, the existing RK3399 description is
> simplified. The CPU backend creates one DTPM zone per cpufreq policy, so
> describing every CPU belonging to the same policy results in duplicate
> zones. The RK3399 hierarchy is therefore reduced to one CPU node per
> cpufreq policy.
> 
> The Rockchip virtual node descriptor is also made SoC-independent so it
> can be reused by other Rockchip platforms.
> 
> Finally, an RK3588 hierarchy is added. The RK3588 exposes three cpufreq
> policies, represented by cpu@0, cpu@400 and cpu@600. These three CPU
> domains are grouped below a virtual package node:
> 
> rk3588
>    `-- package
>      |-- cpu0-cpufreq
>      |-- cpu4-cpufreq
>      `-- cpu6-cpufreq
> 
> 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                       | 165 ++++++------
>   drivers/powercap/dtpm_cpu.c                   |  22 +-
>   drivers/powercap/dtpm_devfreq.c               |  22 +-
>   drivers/powercap/powercap_sys.c               | 148 +++++++++++
>   drivers/soc/rockchip/dtpm.c                   | 127 ++++++---
>   include/linux/dtpm.h                          |   5 +-
>   include/linux/powercap.h                      | 178 +++++++++++++
>   tools/testing/selftests/Makefile              |   1 +
>   tools/testing/selftests/powercap/Kbuild       |   3 +
>   tools/testing/selftests/powercap/Makefile     |  17 ++
>   .../selftests/powercap/powercap_hierarchy.c   | 246 ++++++++++++++++++
>   .../selftests/powercap/powercap_hierarchy.sh  | 118 +++++++++
>   12 files changed, 906 insertions(+), 146 deletions(-)
>   create mode 100644 tools/testing/selftests/powercap/Kbuild
>   create mode 100644 tools/testing/selftests/powercap/Makefile
>   create mode 100644 tools/testing/selftests/powercap/powercap_hierarchy.c
>   create mode 100755 tools/testing/selftests/powercap/powercap_hierarchy.sh
> 

Re: [PATCH v2 1/6] powercap: Add generic zone hierarchy creation helpers

[Rafael J. Wysocki (Intel)]

On Wed, Sep 23, 2026 at 4:34 PM Daniel Lezcano
<daniel.lezcano@oss.qualcomm.com> wrote:
>
> 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 = {
>           .nodes = nodes,
>           .nr_nodes = ARRAY_SIZE(nodes),
>   };
>
> creates the following hierarchy:
>
>   package
>   |-- cpu
>   `-- gpu

I gather that this works, but TBH it doesn't look particularly nice,
especially because of the way in which indices need to be used in it.

If a few extra bytes can be sacrificed for storing extra pointers,
something like this can be done instead:

struct powercap_node {
    const char *name;
    struct powercap_node *parent_node;
    struct powercap_zone *pcap_zone;
    void *data;
    unsigned int nr_subnodes;
    struct powercap_node *subnodes[];
};

struct powercap_node cpu_node = {
    .name = "cpu",
};

struct powercap_node gpu_node = {
    .name = "gpu",
};

struct powercap_node package_node = {
    .name = "package",
    .nr_subnodes = ARRAY_SIZE(package_node.subnodes),
    .subnodes[] = { &cpu_node, &gpu_node };
};

and then &package_node can be passed directly to
powercap_hierarchy_create() without a need to create an auxiliary
struct powercap_hierarchy.

The parent_node pointers can be populated during the creation of the hierarchy.

> Add powercap_hierarchy_dup() to create a runtime copy of a hierarchy
> description. Rebase the parent pointers so that the copy does not keep
> references to the original array, which may be stored in init memory.
>
> Add powercap_hierarchy_create() to walk the description in order
> and create each zone through a controller-provided callback. The parent
> zone is passed to the callback, keeping the node creation operation
> specific to the controller.
>
> The backend is responsible for allocating and registering each powercap
> zone from the creation callback. The powercap_zone object is expected to
> be embedded in a backend-specific structure, allowing the backend
> callbacks to retrieve their private data later using container_of().
>
> For example:
>
>   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);
>
> Its allocation, private state and lifetime remain under the control of
> the backend.
>
> The creation callback therefore returns the same powercap_zone pointer

Which creation callback?

> that was passed to powercap_register_zone(). The hierarchy helper stores
> this pointer to provide it as the parent of subsequent nodes and to pass
> it back to the backend during hierarchy destruction.
>
> 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.
>
> Cc: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> Link: https://patch.msgid.link/20260806110159.69690-2-daniel.lezcano@oss.qualcomm.com
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> ---
>  drivers/powercap/powercap_sys.c | 148 ++++++++++++++++++++++++++
>  include/linux/powercap.h        | 178 ++++++++++++++++++++++++++++++++
>  2 files changed, 326 insertions(+)
>
> diff --git a/drivers/powercap/powercap_sys.c b/drivers/powercap/powercap_sys.c
> index 9197fa20d93f..fefb36c5bcd2 100644
> --- a/drivers/powercap/powercap_sys.c
> +++ b/drivers/powercap/powercap_sys.c
> @@ -667,6 +667,154 @@ 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)

This is sort of awkward even though I seem to understand its purpose.
The duplication is extra overhead that would be entirely avoidable by
creating the original structure in memory that is not __initdata.

> +{
> +       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(&copy->lock);
> +
> +       for (i = 0; i < copy->nr_nodes; i++) {
> +               const struct powercap_node *parent = hierarchy->nodes[i].parent;
> +               ptrdiff_t index;
> +
> +               copy->nodes[i].pcz = 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 = &copy->nodes[index];
> +       }
> +
> +       return copy;
> +
> +invalid:
> +       mutex_destroy(&copy->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)
> +{
> +       size_t i;

ssize_t i;

would be more convenient.

> +
> +       for (i = hierarchy->nr_nodes; i-- > 0;) {

First, if you use for (), there is no need to combine the
decrementation of the index variable with the termination condition.

I'd do

      for (i = hierarchy->nr_nodes - 1; i >= 0; i--) {

Second, this seems to require a certain ordering of rows in the array
representing the hierarchy.  In particular, the index of a parent node
needs to be less than the indices of all of its child nodes AFAICS,
which makes it a bit fragile.  Note that my example arrangement
doesn't have this problem.

> +               if (!hierarchy->nodes[i].pcz)
> +                       continue;
> +
> +               powercap_node_destroy(pct, hierarchy->nodes[i].pcz,
> +                                     hierarchy->nodes[i].data);
> +
> +               hierarchy->nodes[i].pcz = 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;
> +
> +       guard(mutex)(&hierarchy->lock);
> +
> +       __powercap_hierarchy_destroy(pct, hierarchy, powercap_node_destroy);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(powercap_hierarchy_destroy);
> +
> +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;
> +
> +       guard(mutex)(&hierarchy->lock);
> +
> +       for (i = 0; i < hierarchy->nr_nodes; i++) {
> +               struct powercap_zone *parent = NULL;
> +
> +               if (hierarchy->nodes[i].parent) {
> +                       parent = hierarchy->nodes[i].parent->pcz;
> +                       if (!parent) {
> +                               ret = -EINVAL;
> +                               goto rollback;
> +                       }
> +               }
> +
> +               pcz = powercap_node_create(pct, hierarchy->nodes[i].name,
> +                                          hierarchy->nodes[i].data, parent);
> +               if (IS_ERR_OR_NULL(pcz)) {
> +                       ret = pcz ? PTR_ERR(pcz) : -EINVAL;
> +                       goto rollback;
> +               }
> +
> +               hierarchy->nodes[i].pcz = pcz;
> +       }
> +
> +       return 0;
> +
> +rollback:
> +       __powercap_hierarchy_destroy(pct, hierarchy, powercap_node_destroy);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(powercap_hierarchy_create);
> +
>  static int __init powercap_init(void)
>  {
>         int result;
> diff --git a/include/linux/powercap.h b/include/linux/powercap.h
> index 603419db924c..939cd58dff4f 100644
> --- a/include/linux/powercap.h
> +++ b/include/linux/powercap.h
> @@ -9,6 +9,7 @@
>
>  #include <linux/device.h>
>  #include <linux/idr.h>
> +#include <linux/mutex.h>
>
>  /*
>   * A power cap class device can contain multiple powercap control_types.
> @@ -309,4 +310,181 @@ struct powercap_zone *powercap_register_zone(
>  int powercap_unregister_zone(struct powercap_control_type *control_type,
>                                 struct powercap_zone *power_zone);
> +/**
> + * struct powercap_node - Description of a node in a powercap hierarchy
> + * @name: Name of the powercap zone.
> + * @parent: Parent node, or NULL if the node is a hierarchy root.
> + * @pcz: Powercap zone created for this node. This field is managed by the
> + *      hierarchy creation and destruction helpers.
> + * @data: Private data passed unchanged to the creation and destruction
> + *       callbacks.
> + *
> + * This structure describes one node of a powercap hierarchy. The backend
> + * supplies an array of nodes through &struct powercap_hierarchy.
> + *
> + * Nodes must be ordered so that a parent appears before all its children.
> + *
> + * The @name and @data objects must remain valid until the duplicated hierarchy
> + * has been freed. The @parent pointer is rebased by powercap_hierarchy_dup(),
> + * allowing the original node array to be released after duplication.
> + *
> + * The @pcz field is runtime state managed by powercap_hierarchy_create() and
> + * powercap_hierarchy_destroy(). It is cleared when a node is destroyed.
> + */
> +struct powercap_node {
> +       const char *name;
> +       struct powercap_node *parent;
> +       struct powercap_zone *pcz;

I'd call this "zone" or "pcap_zone" like in my example above.

> +       void *data;
> +};
> +
> +/**
> + * struct powercap_hierarchy - Powercap zone hierarchy
> + * @nodes: Array describing the hierarchy nodes.
> + * @nr_nodes: Number of entries in @nodes.
> + * @lock: Lock serializing hierarchy creation and destruction.
> + *
> + * A backend may place the initial description in init memory and duplicate it
> + * with powercap_hierarchy_dup() before the init sections are released. The
> + * duplicated hierarchy owns the @nodes array, but not the objects referenced
> + * by &struct powercap_node.name and &struct powercap_node.data.
> + */
> +struct powercap_hierarchy {
> +       struct powercap_node *nodes;
> +       size_t nr_nodes;
> +       struct mutex lock;
> +};
> +

I'd prefer the kerneldoc comments of new functions to go where the
function definitions are located.

> +/**
> + * powercap_hierarchy_dup - Duplicate a powercap hierarchy description
> + * @hierarchy: Hierarchy description to duplicate.
> + *
> + * Allocate a runtime hierarchy and copy the nodes from @hierarchy. Parent
> + * pointers are rebased to the duplicated node array and the runtime pcz fields
> + * are initialized to NULL.
> + *
> + * Every parent must belong to the source node array and precede its children.
> + * The node names and private data are not duplicated and must remain valid
> + * until powercap_hierarchy_free() is called.
> + *
> + * Context: Process context. May sleep.
> + *
> + * Return: A pointer to the duplicated hierarchy on success, or an ERR_PTR()
> + * encoded error otherwise.
> + */
> +struct powercap_hierarchy *powercap_hierarchy_dup(const struct powercap_hierarchy *hierarchy);
> +
> +/**
> + * powercap_hierarchy_free - Free a duplicated powercap hierarchy
> + * @hierarchy: Hierarchy to free, or NULL.
> + *
> + * Free the node array and hierarchy allocated by powercap_hierarchy_dup(). All
> + * registered zones must have been destroyed before calling this function.
> + */
> +void powercap_hierarchy_free(struct powercap_hierarchy *hierarchy);
> +
> +/**
> + * typedef powercap_node_create_t - Create a powercap hierarchy node
> + * @pct: Powercap control type owning the hierarchy.
> + * @name: Name of the powercap zone to create.
> + * @data: Private data associated with the hierarchy node.
> + * @parent: Parent powercap zone, or NULL for a root node.
> + *
> + * Callback invoked by powercap_hierarchy_create() for each node in the
> + * hierarchy. The callback must create and register a powercap zone below
> + * @parent. The backend is expected to embed struct powercap_zone in its own
> + * object, pass the address of that member to powercap_register_zone(), and
> + * return the same address from this callback. This lets the backend recover
> + * its object with container_of() from subsequent powercap callbacks.
> + *
> + * Context: Called with the powercap hierarchy mutex held. The callback may
> + * sleep, but must not call powercap_hierarchy_create() or
> + * powercap_hierarchy_destroy() for the same hierarchy.
> + *
> + * Return: A valid pointer to the created powercap zone on success, or an
> + * ERR_PTR() encoded error on failure.
> + */
> +typedef struct powercap_zone *(*powercap_node_create_t)(struct powercap_control_type *pct,
> +                                                       const char *name, void *data,
> +                                                       struct powercap_zone *parent);
> +/**
> + * typedef powercap_node_destroy_t - Destroy a powercap hierarchy node
> + * @pct: Powercap control type owning the hierarchy.
> + * @zone: Powercap zone to destroy.
> + * @data: Private data associated with the hierarchy node.
> + *
> + * Callback invoked when a hierarchy is destroyed or when its creation must
> + * be rolled back. The callback must unregister the powercap zone represented
> + * by @zone. The backend remains responsible for the lifetime of the enclosing
> + * object, including releasing it from the powercap zone release callback when
> + * necessary.
> + *
> + * Nodes are passed to this callback in reverse creation order, ensuring that
> + * all children are destroyed before their parent.
> + *
> + * Context: Called with the powercap hierarchy mutex held. The callback may
> + * sleep, but must not call powercap_hierarchy_create() or
> + * powercap_hierarchy_destroy() for the same hierarchy.
> + */
> +typedef void (*powercap_node_destroy_t)(struct powercap_control_type *pct,
> +                                       struct powercap_zone *zone,
> +                                       void *data);
> +
> +/**
> + * powercap_hierarchy_destroy - Destroy a powercap hierarchy
> + * @pct: Powercap control type owning the hierarchy.
> + * @hierarchy: Hierarchy to destroy.
> + * @powercap_node_destroy: Callback used to destroy each powercap zone.
> + *
> + * Destroy all powercap zones previously created for @hierarchy. Nodes are
> + * destroyed in reverse array order so that children are removed before their
> + * parents.
> + *
> + * Entries whose &struct powercap_node.pcz field is NULL are ignored. After
> + * a zone has been destroyed, its pcz field is cleared.
> + *
> + * The caller must ensure that no users of the hierarchy remain when this
> + * function is called.
> + *
> + * Context: Process context. May sleep.
> + *
> + * Return: 0 on success or -EINVAL if an argument is invalid.
> + */
> +int powercap_hierarchy_destroy(struct powercap_control_type *pct,
> +                              struct powercap_hierarchy *hierarchy,
> +                              powercap_node_destroy_t powercap_node_destroy);
> +
> +/**
> + * powercap_hierarchy_create - Create a powercap hierarchy
> + * @pct: Powercap control type that will own the hierarchy.
> + * @hierarchy: Hierarchy to create.
> + * @powercap_node_create: Callback used to create each powercap zone.
> + * @powercap_node_destroy: Callback used to roll back an incomplete hierarchy.
> + *
> + * Create the powercap zones described by @hierarchy in array order. For each
> + * entry, @powercap_node_create is called with the powercap zone stored in its
> + * parent entry. Root nodes are created with a NULL parent.
> + *
> + * Each parent entry must precede all its children in the node array. All pcz
> + * fields must be NULL when this function is called.
> + *
> + * If a node cannot be created, all zones created by this invocation are
> + * destroyed in reverse order by calling @powercap_node_destroy. Therefore,
> + * @powercap_node_destroy must be provided even when the caller does not
> + * expect to destroy the hierarchy explicitly.
> + *
> + * The hierarchy and the objects referenced by its name and data fields must
> + * remain valid until powercap_hierarchy_destroy() has completed.
> + *
> + * Context: Process context. May sleep.
> + *
> + * Return: 0 on success, -EINVAL if an argument or hierarchy entry is invalid,
> + * -EBUSY if the hierarchy already contains a created zone, or the error
> + * returned by @powercap_node_create.
> + */
> +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);
> +
>  #endif
> --