[PATCH 0/3] Add Qualcomm SPEL powercap driver

[PATCH 0/3] Add Qualcomm SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

This patch series introduces support for Qualcomm's SPEL (SoC Power
and Electrical Limits) hardware, which provides power monitoring and
limiting capabilities for various power domains in Qualcomm SoCs.

The SPEL hardware enables:
- Real-time energy consumption monitoring across multiple power domains
- Hardware-enforced power limits with configurable time windows
- Support for up to 4 power limits (PL1-PL4) per domain
- Hierarchical power domain organization (SYS -> SOC -> subsystems)

The driver integrates with the Linux powercap framework, exposing SPEL
capabilities through standard sysfs interfaces. This allows userspace
applications and thermal management daemons to monitor energy consumption
and configure power limits for optimal power/performance balance.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
Manaf Meethalavalappu Pallikunhi (3):
      dt-bindings: power: limits: Describe Qualcomm SPEL hardware
      powercap: qcom: Add SPEL powercap driver
      arm64: dts: qcom: glymur: Enable SPEL powercap driver

 .../bindings/power/limits/qcom,spel.yaml           |  47 ++
 MAINTAINERS                                        |   7 +
 arch/arm64/boot/dts/qcom/glymur.dtsi               |  10 +
 drivers/powercap/Kconfig                           |  13 +
 drivers/powercap/Makefile                          |   1 +
 drivers/powercap/qcom_spel.c                       | 787 +++++++++++++++++++++
 6 files changed, 865 insertions(+)
---
base-commit: 80dd246accce631c328ea43294e53b2b2dd2aa32
change-id: 20260519-qcom_spel_driver_upstream-11efd8c85e6e

Best regards,
-- 
Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>

[PATCH 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware

[Manaf Meethalavalappu Pallikunhi]

The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
based power monitoring and limiting capabilities for various domains.

Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 .../bindings/power/limits/qcom,spel.yaml           | 47 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 53 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
new file mode 100644
index 000000000000..e9bb70c3a3aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/limits/qcom,spel.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SoC Power and Electrical Limits (SPEL)
+
+maintainers:
+  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
+
+description:
+  The Qualcomm SPEL (SoC Power and Electrical Limits) provides hardware-based
+  power monitoring and limiting capabilities for various power domains in
+  Qualcomm SoCs.
+
+properties:
+  compatible:
+    const: qcom,spel
+
+  reg:
+    maxItems: 3
+
+  reg-names:
+    items:
+      - const: nodes
+      - const: constraints
+      - const: config
+
+required:
+  - compatible
+  - reg
+  - reg-names
+
+additionalProperties: false
+
+examples:
+  - |
+    power-controller@ef3b000 {
+        compatible = "qcom,spel";
+        reg = <0x0ef3e000 0x1000>,
+              <0x0ef3d000 0x1000>,
+              <0x0ef3b000 0x1000>;
+        reg-names = "nodes",
+                    "constraints",
+                    "config";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3dd58a16f06a..c63f147e8c54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22233,6 +22233,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/power/supply/qcom,pmi8998-charger.yaml
 F:	drivers/power/supply/qcom_smbx.c
 
+QUALCOMM SPEL POWERCAP DRIVER
+M:	Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
+
 QUALCOMM PPE DRIVER
 M:	Luo Jie <quic_luoj@quicinc.com>
 L:	netdev@vger.kernel.org

-- 
2.43.0

Re: [PATCH 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The unit-address in the example node name does not match the first address in the `reg` property.
--

commit f3971a9e10b2466608195c2613f6957464048b20
Author: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>

dt-bindings: power: limits: Describe Qualcomm SPEL hardware

This commit adds a DeviceTree binding to describe the SoC Power and Electrical
Limits (SPEL) block on Qualcomm SoCs.

> diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml

[ ... ]

> +examples:
> +  - |
> +    power-controller@ef3b000 {
> +        compatible = "qcom,spel";
> +        reg = <0x0ef3e000 0x1000>,

This isn't a bug, but does the unit-address in the example node name match
the first address in the reg property? 

It looks like the node is named power-controller@ef3b000, but the first
address in the reg property is 0x0ef3e000. This mismatch will likely trigger
a dtc warning during dt_binding_check.

> +              <0x0ef3d000 0x1000>,
> +              <0x0ef3b000 0x1000>;
> +        reg-names = "nodes",
> +                    "constraints",
> +                    "config";
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-qcom_spel_driver_upstream-v1-0-75356d1b7f94@oss.qualcomm.com?part=1

Re: [PATCH 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware

[Rob Herring (Arm)]

On Tue, 19 May 2026 16:19:49 +0530, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various domains.
> 
> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  .../bindings/power/limits/qcom,spel.yaml           | 47 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 53 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/limits/qcom,spel.example.dtb: power-controller@ef3b000 (qcom,spel): '#power-domain-cells' is a required property
	from schema $id: http://devicetree.org/schemas/power/power-domain.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260519-qcom_spel_driver_upstream-v1-1-75356d1b7f94@oss.qualcomm.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware

[Manaf Meethalavalappu Pallikunhi]

Hi Rob,

On 5/19/2026 11:10 PM, Rob Herring (Arm) wrote:
> 
> On Tue, 19 May 2026 16:19:49 +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>> based power monitoring and limiting capabilities for various domains.
>>
>> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   .../bindings/power/limits/qcom,spel.yaml           | 47 ++++++++++++++++++++++
>>   MAINTAINERS                                        |  6 +++
>>   2 files changed, 53 insertions(+)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/limits/qcom,spel.example.dtb: power-controller@ef3b000 (qcom,spel): '#power-domain-cells' is a required property
> 	from schema $id: http://devicetree.org/schemas/power/power-domain.yaml
> 
> doc reference errors (make refcheckdocs):

Ack, Will move node name to power-limits instead of power-controller

Thanks,
Manaf

> 
> See https://patchwork.kernel.org/project/devicetree/patch/20260519-qcom_spel_driver_upstream-v1-1-75356d1b7f94@oss.qualcomm.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

Re: [PATCH 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware

[Krzysztof Kozlowski]

On 19/05/2026 12:49, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various domains.
> 
> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  .../bindings/power/limits/qcom,spel.yaml           | 47 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
> new file mode 100644
> index 000000000000..e9bb70c3a3aa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/limits/qcom,spel.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SoC Power and Electrical Limits (SPEL)
> +
> +maintainers:
> +  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> +
> +description:
> +  The Qualcomm SPEL (SoC Power and Electrical Limits) provides hardware-based
> +  power monitoring and limiting capabilities for various power domains in
> +  Qualcomm SoCs.
> +
> +properties:
> +  compatible:
> +    const: qcom,spel

You don't get generic compatibles. I think I commented about this enough
of times.

NAK

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware

[Manaf Meethalavalappu Pallikunhi]

Hi Krzysztof,


On 5/30/2026 5:59 PM, Krzysztof Kozlowski wrote:
> On 19/05/2026 12:49, Manaf Meethalavalappu Pallikunhi wrote:
>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>> based power monitoring and limiting capabilities for various domains.
>>
>> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   .../bindings/power/limits/qcom,spel.yaml           | 47 ++++++++++++++++++++++
>>   MAINTAINERS                                        |  6 +++
>>   2 files changed, 53 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
>> new file mode 100644
>> index 000000000000..e9bb70c3a3aa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
>> @@ -0,0 +1,47 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/limits/qcom,spel.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SoC Power and Electrical Limits (SPEL)
>> +
>> +maintainers:
>> +  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> +
>> +description:
>> +  The Qualcomm SPEL (SoC Power and Electrical Limits) provides hardware-based
>> +  power monitoring and limiting capabilities for various power domains in
>> +  Qualcomm SoCs.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,spel
> 
> You don't get generic compatibles. I think I commented about this enough
> of times.

ACK, I will move to platform specific compatibles in V2.

Thanks,
Manaf>
> NAK
> 
> Best regards,
> Krzysztof

[PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
based power monitoring and limiting capabilities for various power
domains including System, SoC, CPU clusters, GPU, and various other
subsystems.

The driver integrates with the Linux powercap framework, exposing SPEL
capabilities through powercap sysfs interfaces.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 MAINTAINERS                  |   1 +
 drivers/powercap/Kconfig     |  13 +
 drivers/powercap/Makefile    |   1 +
 drivers/powercap/qcom_spel.c | 787 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 802 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c63f147e8c54..5c7542754ab6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22238,6 +22238,7 @@ M:	Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
 L:	linux-arm-msm@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
+F:	drivers/powercap/qcom_spel.c
 
 QUALCOMM PPE DRIVER
 M:	Luo Jie <quic_luoj@quicinc.com>
diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
index 03c4c796d993..e3a47c653499 100644
--- a/drivers/powercap/Kconfig
+++ b/drivers/powercap/Kconfig
@@ -93,4 +93,17 @@ config DTPM_DEVFREQ
 	help
 	  This enables support for device power limitation based on
 	  energy model.
+
+config QCOM_SPEL
+	tristate "Qualcomm SPEL Powercap driver"
+	depends on ARM64 || COMPILE_TEST
+	help
+	  This enables support for the Qualcomm SoC Power and Electrical
+	  Limits (SPEL) hardware, which allows power limits to be
+	  enforced and monitored on Qualcomm SoCs.
+
+	  SPEL provides energy monitoring and power capping for multiple
+	  domains including system, SoC, CPU clusters, GPU, and various
+	  other subsystems.
+
 endif
diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile
index 5ab0dce565b9..8235fb9d3df6 100644
--- a/drivers/powercap/Makefile
+++ b/drivers/powercap/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_INTEL_RAPL) += intel_rapl_msr.o
 obj-$(CONFIG_INTEL_RAPL_TPMI) += intel_rapl_tpmi.o
 obj-$(CONFIG_IDLE_INJECT) += idle_inject.o
 obj-$(CONFIG_ARM_SCMI_POWERCAP) += arm_scmi_powercap.o
+obj-$(CONFIG_QCOM_SPEL) += qcom_spel.o
diff --git a/drivers/powercap/qcom_spel.c b/drivers/powercap/qcom_spel.c
new file mode 100644
index 000000000000..fed5647959a5
--- /dev/null
+++ b/drivers/powercap/qcom_spel.c
@@ -0,0 +1,787 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm SPEL (SoC Power and Electrical Limits) Driver
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/bitmap.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/powercap.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+/* SPEL register bitmasks */
+#define ENERGY_STATUS_MASK		0xFFFFFFFF
+
+#define POWER_LIMIT_MASK		0x00007FFF
+#define POWER_LIMIT_ENABLE		BIT(31)
+
+#define TIME_WINDOW_MASK_L		0x00007FFF	/* bits [14:0] */
+#define TIME_WINDOW_MASK_H		0x007F0000	/* bits [22:16] */
+
+#define ENERGY_UNIT_OFFSET		16
+#define ENERGY_UNIT_MASK		0xF0000
+
+#define TIME_UNIT_OFFSET		8
+#define TIME_UNIT_MASK			0xF00
+
+#define POWER_UNIT_OFFSET		0
+#define POWER_UNIT_MASK			0x7
+
+#define LIMITS_CAPABILITY_OFFSET	0x20
+#define ENERGY_RPT_UNIT_OFFSET		0x04
+
+#define ENERGY_UNIT_SCALE		1000
+
+#define SPEL_DOMAIN_NAME_LENGTH		16
+
+/* Domain types */
+enum spel_domain_type {
+	SPEL_DOMAIN_SYS,
+	SPEL_DOMAIN_SOC,
+	SPEL_DOMAIN_CL0,
+	SPEL_DOMAIN_CL1,
+	SPEL_DOMAIN_CL2,
+	SPEL_DOMAIN_IGPU,
+	SPEL_DOMAIN_DGPU,
+	SPEL_DOMAIN_NSP,
+	SPEL_DOMAIN_MMCX,
+	SPEL_DOMAIN_INFRA,
+	SPEL_DOMAIN_DRAM,
+	SPEL_DOMAIN_MDM,
+	SPEL_DOMAIN_WLAN,
+	SPEL_DOMAIN_USB1,
+	SPEL_DOMAIN_USB2,
+	SPEL_DOMAIN_USB3,
+	SPEL_DOMAIN_MAX,
+};
+
+/* Power limit IDs */
+enum spel_power_limit_id {
+	POWER_LIMIT1,
+	POWER_LIMIT2,
+	POWER_LIMIT3,
+	POWER_LIMIT4,
+	NR_POWER_LIMITS,
+};
+
+/* Unit types for conversion */
+enum unit_type {
+	POWER_UNIT,
+	ENERGY_UNIT,
+	TIME_UNIT,
+};
+
+/* Power limit operation types */
+enum pl_ops_type {
+	PL_LIMIT,
+	PL_TIME_WINDOW,
+};
+
+static const char *pl_names[NR_POWER_LIMITS] = {
+	[POWER_LIMIT1] = "pl1",
+	[POWER_LIMIT2] = "pl2",
+	[POWER_LIMIT3] = "pl3",
+	[POWER_LIMIT4] = "pl4",
+};
+
+static const char *const spel_domain_names[] = {
+	"sys", "soc", "cl0", "cl1", "cl2", "igpu", "dgpu", "nsp",
+	"mmcx", "infra", "dram", "mdm", "wlan", "usb1", "usb2", "usb3",
+};
+
+/* Domain register offsets in node base */
+static const u32 domain_offsets[SPEL_DOMAIN_MAX] = {
+	[SPEL_DOMAIN_SYS]	= 0x40,
+	[SPEL_DOMAIN_SOC]	= 0x00,
+	[SPEL_DOMAIN_CL0]	= 0x5C,
+	[SPEL_DOMAIN_CL1]	= 0x60,
+	[SPEL_DOMAIN_CL2]	= 0x64,
+	[SPEL_DOMAIN_IGPU]	= 0x08,
+	[SPEL_DOMAIN_DGPU]	= 0x44,
+	[SPEL_DOMAIN_NSP]	= 0x0C,
+	[SPEL_DOMAIN_MMCX]	= 0x10,
+	[SPEL_DOMAIN_INFRA]	= 0x18,
+	[SPEL_DOMAIN_DRAM]	= 0x1C,
+	[SPEL_DOMAIN_MDM]	= 0x48,
+	[SPEL_DOMAIN_WLAN]	= 0x4C,
+	[SPEL_DOMAIN_USB1]	= 0x50,
+	[SPEL_DOMAIN_USB2]	= 0x54,
+	[SPEL_DOMAIN_USB3]	= 0x58,
+};
+
+/**
+ * struct spel_constraint_info - Power limit constraint information
+ * @limit_offset:	Register offset for power limit value
+ * @time_window_offset:	Register offset for time window
+ * @supported_mask:	Bit mask in capability register
+ * @domain_id:		Domain this constraint applies to
+ * @pl_id:		Power limit ID (PL1, PL2, etc.)
+ */
+struct spel_constraint_info {
+	u32 limit_offset;
+	u32 time_window_offset;
+	u32 supported_mask;
+	enum spel_domain_type domain_id;
+	int pl_id;
+};
+
+/* Constraint configuration */
+static struct spel_constraint_info constraints[] = {
+	/* SYS domain constraints */
+	{ 0x10, 0x70, BIT(0), SPEL_DOMAIN_SYS, POWER_LIMIT1 },
+	{ 0x14, 0x74, BIT(1), SPEL_DOMAIN_SYS, POWER_LIMIT2 },
+	{ 0x18, 0x78, BIT(2), SPEL_DOMAIN_SYS, POWER_LIMIT3 },
+	{ 0x1C, 0x7C, BIT(3), SPEL_DOMAIN_SYS, POWER_LIMIT4 },
+	/* SOC domain constraints */
+	{ 0x00, 0x60, BIT(4), SPEL_DOMAIN_SOC, POWER_LIMIT1 },
+	{ 0x04, 0x64, BIT(5), SPEL_DOMAIN_SOC, POWER_LIMIT2 },
+	{ 0x08, 0x68, BIT(6), SPEL_DOMAIN_SOC, POWER_LIMIT3 },
+	{ 0x0C, 0x6C, BIT(7), SPEL_DOMAIN_SOC, POWER_LIMIT4 },
+};
+
+struct spel_system;
+
+/**
+ * struct spel_domain - SPEL power domain
+ * @power_zone:		Powercap zone
+ * @lock:		Mutex protecting register access
+ * @sp:			Parent system
+ * @status_reg:		Energy counter register
+ * @pl_name:		Power limit names
+ * @name:		Domain name
+ * @id:			Domain type ID
+ */
+struct spel_domain {
+	struct powercap_zone power_zone;
+	struct mutex lock; /* Protects register read/write operations */
+	struct spel_system *sp;
+	void __iomem *status_reg;
+	const char *pl_name[NR_POWER_LIMITS];
+	char name[SPEL_DOMAIN_NAME_LENGTH];
+	enum spel_domain_type id;
+};
+
+/**
+ * struct spel_system -	SPEL system
+ * @domains:		Array of domains
+ * @power_zone:		Parent powercap zone
+ * @node_base:		Base address for node registers
+ * @constraint_base:	Base address for constraint registers
+ * @config_base:	Base address for config registers
+ * @control_type:	Powercap control type
+ * @dev:		Device pointer for logging
+ * @limits:		Supported power limits per domain
+ * @power_unit:		Power unit in microWatts (common for all domains)
+ * @energy_unit:	Energy unit in nanoJoules (common for all domains)
+ * @time_unit:		Time unit in microseconds (common for all domains)
+ */
+struct spel_system {
+	struct spel_domain *domains;
+	struct powercap_zone *power_zone;
+	void __iomem *node_base;
+	void __iomem *constraint_base;
+	void __iomem *config_base;
+	struct powercap_control_type *control_type;
+	struct device *dev;
+	int limits[SPEL_DOMAIN_MAX];
+	unsigned int power_unit;
+	unsigned int energy_unit;
+	unsigned int time_unit;
+};
+
+#define power_zone_to_spel_domain(_zone) \
+	container_of(_zone, struct spel_domain, power_zone)
+
+/* Helper functions */
+static bool is_pl_valid(struct spel_domain *sd, int pl)
+{
+	if (pl < POWER_LIMIT1 || pl >= NR_POWER_LIMITS)
+		return false;
+	return sd->pl_name[pl] ? true : false;
+}
+
+static int get_pl_ops_offset(struct spel_domain *sd, int pl, enum pl_ops_type pl_op)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(constraints); i++) {
+		struct spel_constraint_info *ci = &constraints[i];
+
+		if (ci->domain_id == sd->id && ci->pl_id == pl) {
+			switch (pl_op) {
+			case PL_LIMIT:
+				return ci->limit_offset;
+			case PL_TIME_WINDOW:
+				return ci->time_window_offset;
+			default:
+				return -EOPNOTSUPP;
+			}
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static u64 spel_unit_xlate(struct spel_domain *sd, enum unit_type type,
+			   u64 value, int to_raw)
+{
+	struct spel_system *sp = sd->sp;
+	u64 units = 1;
+	u64 scale = 1;
+
+	switch (type) {
+	case POWER_UNIT:
+		units = sp->power_unit;
+		break;
+	case ENERGY_UNIT:
+		scale = ENERGY_UNIT_SCALE;
+		units = sp->energy_unit;
+		break;
+	case TIME_UNIT:
+		units = sp->time_unit;
+		break;
+	default:
+		return value;
+	}
+
+	if (to_raw)
+		return div64_u64(value * scale, units);
+
+	value *= units;
+	return div64_u64(value, scale);
+}
+
+/* Power limit data access */
+static int spel_read_pl_data(struct spel_domain *sd, int pl,
+			     enum pl_ops_type pl_op, bool xlate, u64 *data)
+{
+	struct spel_system *sp = sd->sp;
+	void __iomem *reg_addr;
+	u64 value;
+	int offset;
+
+	if (!is_pl_valid(sd, pl))
+		return -EINVAL;
+
+	offset = get_pl_ops_offset(sd, pl, pl_op);
+	if (offset < 0)
+		return offset;
+
+	guard(mutex)(&sd->lock);
+
+	reg_addr = sp->constraint_base + offset;
+	value = readl(reg_addr);
+
+	switch (pl_op) {
+	case PL_LIMIT:
+		value &= POWER_LIMIT_MASK;
+		if (xlate)
+			*data = spel_unit_xlate(sd, POWER_UNIT, value, 0);
+		else
+			*data = value;
+		break;
+	case PL_TIME_WINDOW:
+		/* Decode time window: bits [22:16] are upper 7 bits, [14:0] are lower 15 bits */
+		value = ((value & TIME_WINDOW_MASK_H) >> 16 << 15) |
+			(value & TIME_WINDOW_MASK_L);
+		if (xlate)
+			*data = spel_unit_xlate(sd, TIME_UNIT, value, 0);
+		else
+			*data = value;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int spel_write_pl_data(struct spel_domain *sd, int pl,
+			      enum pl_ops_type pl_op, unsigned long long value)
+{
+	struct spel_system *sp = sd->sp;
+	void __iomem *reg_addr;
+	u64 reg_val, new_val;
+	int offset;
+
+	if (!is_pl_valid(sd, pl))
+		return -EINVAL;
+
+	offset = get_pl_ops_offset(sd, pl, pl_op);
+	if (offset < 0)
+		return offset;
+
+	guard(mutex)(&sd->lock);
+
+	reg_addr = sp->constraint_base + offset;
+	reg_val = readl(reg_addr);
+
+	switch (pl_op) {
+	case PL_LIMIT:
+		new_val = spel_unit_xlate(sd, POWER_UNIT, value, 1);
+		if (new_val > POWER_LIMIT_MASK)
+			return -EINVAL;
+		reg_val = (reg_val & ~POWER_LIMIT_MASK) | new_val;
+
+		/*
+		 * Enable/Disable PL based on the value:
+		 * - If value is 0, disable the PL (clear enable bit)
+		 * - If value is non-zero, enable the PL (set enable bit)
+		 */
+		if (new_val == 0)
+			reg_val &= ~POWER_LIMIT_ENABLE;
+		else
+			reg_val |= POWER_LIMIT_ENABLE;
+		break;
+	case PL_TIME_WINDOW:
+		/*
+		 * Encode time window: upper 7 bits to [22:16], lower 15 bits to [14:0]
+		 * Time window register is separate from limit register (different offset),
+		 * so we write only the time window bits without preserving any enable bit.
+		 */
+		new_val = spel_unit_xlate(sd, TIME_UNIT, value, 1);
+		reg_val = (((new_val >> 15) & 0x7F) << 16) |
+			  (new_val & 0x7FFF);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	writel(reg_val, reg_addr);
+	return 0;
+}
+
+/* Powercap zone operations */
+static int spel_get_energy_counter(struct powercap_zone *power_zone, u64 *energy_raw)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
+	u64 value;
+
+	value = readl(sd->status_reg);
+	*energy_raw = spel_unit_xlate(sd, ENERGY_UNIT, value, 0);
+
+	return 0;
+}
+
+static int spel_get_max_energy_counter(struct powercap_zone *pcd_dev, u64 *energy)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(pcd_dev);
+
+	*energy = spel_unit_xlate(sd, ENERGY_UNIT, ENERGY_STATUS_MASK, 0);
+	return 0;
+}
+
+static int spel_release_zone(struct powercap_zone *power_zone)
+{
+	return 0;
+}
+
+static int spel_find_nr_power_limit(struct spel_domain *sd)
+{
+	int i, nr_pl = 0;
+
+	for (i = 0; i < NR_POWER_LIMITS; i++) {
+		if (is_pl_valid(sd, i))
+			nr_pl++;
+	}
+
+	return nr_pl;
+}
+
+static const struct powercap_zone_ops zone_ops = {
+	.get_energy_uj = spel_get_energy_counter,
+	.get_max_energy_range_uj = spel_get_max_energy_counter,
+	.release = spel_release_zone,
+};
+
+/* Constraint operations */
+static int spel_constraint_to_pl(struct spel_domain *sd, int cid)
+{
+	int i, j;
+
+	for (i = POWER_LIMIT1, j = 0; i < NR_POWER_LIMITS; i++) {
+		if (is_pl_valid(sd, i) && j++ == cid)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static int spel_set_power_limit(struct powercap_zone *power_zone, int cid,
+				u64 power_limit)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
+	int id;
+
+	id = spel_constraint_to_pl(sd, cid);
+	if (id < 0)
+		return id;
+
+	return spel_write_pl_data(sd, id, PL_LIMIT, power_limit);
+}
+
+static int spel_get_power_limit(struct powercap_zone *power_zone, int cid,
+				u64 *data)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
+	u64 val;
+	int ret, id;
+
+	id = spel_constraint_to_pl(sd, cid);
+	if (id < 0)
+		return id;
+
+	ret = spel_read_pl_data(sd, id, PL_LIMIT, true, &val);
+	if (!ret)
+		*data = val;
+
+	return ret;
+}
+
+static int spel_set_time_window(struct powercap_zone *power_zone, int cid,
+				u64 window)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
+	int id;
+
+	id = spel_constraint_to_pl(sd, cid);
+	if (id < 0)
+		return id;
+
+	return spel_write_pl_data(sd, id, PL_TIME_WINDOW, window);
+}
+
+static int spel_get_time_window(struct powercap_zone *power_zone, int cid,
+				u64 *data)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
+	u64 val;
+	int ret, id;
+
+	id = spel_constraint_to_pl(sd, cid);
+	if (id < 0)
+		return id;
+
+	ret = spel_read_pl_data(sd, id, PL_TIME_WINDOW, true, &val);
+	if (!ret)
+		*data = val;
+
+	return ret;
+}
+
+static const char *spel_get_constraint_name(struct powercap_zone *power_zone,
+					    int cid)
+{
+	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
+	int id;
+
+	id = spel_constraint_to_pl(sd, cid);
+	if (id >= 0)
+		return sd->pl_name[id];
+
+	return NULL;
+}
+
+static const struct powercap_zone_constraint_ops constraint_ops = {
+	.set_power_limit_uw = spel_set_power_limit,
+	.get_power_limit_uw = spel_get_power_limit,
+	.set_time_window_us = spel_set_time_window,
+	.get_time_window_us = spel_get_time_window,
+	.get_name = spel_get_constraint_name,
+};
+
+static void spel_init_domains(struct spel_system *sp)
+{
+	unsigned int i;
+
+	for (i = 0; i < SPEL_DOMAIN_MAX; i++) {
+		struct spel_domain *sd = &sp->domains[i];
+
+		sd->sp = sp;
+		snprintf(sd->name, SPEL_DOMAIN_NAME_LENGTH, "%s",
+			 spel_domain_names[i]);
+		sd->id = i;
+		sd->status_reg = sp->node_base + domain_offsets[i];
+
+		/* PL1 is always supported (required for powercap registration) */
+		sp->limits[i] = BIT(POWER_LIMIT1);
+		sd->pl_name[POWER_LIMIT1] = pl_names[POWER_LIMIT1];
+	}
+}
+
+static int spel_check_unit(struct spel_system *sp)
+{
+	u32 value, shift;
+
+	/* Read power_unit and time_unit from offset 0x0 */
+	value = readl(sp->config_base);
+
+	/*
+	 * Unit calculation: 1 / (2^shift)
+	 * Masks limit: TIME_UNIT (4 bits, max 15), POWER_UNIT (3 bits, max 7).
+	 */
+	shift = (value & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
+	sp->power_unit = 1000000 / (1 << shift);
+
+	shift = (value & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
+	sp->time_unit = 1000000 / (1 << shift);
+
+	/* Read energy_unit from ENERGY_RPT_UNIT_OFFSET */
+	value = readl(sp->config_base + ENERGY_RPT_UNIT_OFFSET);
+
+	/*
+	 * Unit calculation: 1 / (2^shift)
+	 * Masks limit: ENERGY_UNIT (4 bits, max 15).
+	 */
+	shift = (value & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
+	sp->energy_unit = ENERGY_UNIT_SCALE * 1000000 / (1 << shift);
+
+	dev_dbg(sp->dev, "Units: energy=%dnJ, time=%dus, power=%duW\n",
+		sp->energy_unit, sp->time_unit, sp->power_unit);
+
+	return 0;
+}
+
+static void spel_detect_powerlimit(struct spel_domain *sd)
+{
+	struct spel_system *sp = sd->sp;
+	u32 capabilities;
+	int i, j;
+
+	capabilities = readl(sp->config_base + LIMITS_CAPABILITY_OFFSET);
+
+	/* Detect power limits from hardware capabilities */
+	for (i = POWER_LIMIT2; i < NR_POWER_LIMITS; i++) {
+		for (j = 0; j < ARRAY_SIZE(constraints); j++) {
+			struct spel_constraint_info *ci = &constraints[j];
+
+			if (ci->domain_id == sd->id && ci->pl_id == i) {
+				if (capabilities & ci->supported_mask) {
+					sp->limits[sd->id] |= BIT(i);
+					sd->pl_name[i] = pl_names[i];
+				}
+				break;
+			}
+		}
+	}
+}
+
+static int spel_init_system(struct spel_system *sp, struct device *dev)
+{
+	int i, ret;
+
+	/* Read unit configuration (common for all domains) */
+	ret = spel_check_unit(sp);
+	if (ret) {
+		dev_err(dev, "Failed to read unit config\n");
+		return ret;
+	}
+
+	sp->domains = devm_kcalloc(dev, SPEL_DOMAIN_MAX,
+				   sizeof(struct spel_domain), GFP_KERNEL);
+	if (!sp->domains)
+		return -ENOMEM;
+
+	spel_init_domains(sp);
+
+	for (i = 0; i < SPEL_DOMAIN_MAX; i++) {
+		struct spel_domain *sd = &sp->domains[i];
+
+		ret = devm_mutex_init(dev, &sd->lock);
+		if (ret) {
+			dev_err(dev, "Failed to initialize mutex for domain %s\n", sd->name);
+			return ret;
+		}
+
+		spel_detect_powerlimit(sd);
+	}
+
+	return 0;
+}
+
+static int spel_register_powercap(struct spel_system *sp)
+{
+	struct spel_domain *sd;
+	struct powercap_zone *power_zone = NULL;
+	int nr_pl, ret, i;
+
+	/* Register SYS domain as parent zone */
+	for (sd = sp->domains; sd < sp->domains + SPEL_DOMAIN_MAX; sd++) {
+		if (sd->id == SPEL_DOMAIN_SYS) {
+			nr_pl = spel_find_nr_power_limit(sd);
+
+			power_zone = powercap_register_zone(&sd->power_zone,
+							    sp->control_type, sd->name,
+					NULL, &zone_ops, nr_pl,
+					&constraint_ops);
+			if (IS_ERR(power_zone)) {
+				dev_err(sp->dev, "Failed to register power zone %s\n",
+					sd->name);
+				return PTR_ERR(power_zone);
+			}
+			sp->power_zone = power_zone;
+			break;
+		}
+	}
+
+	if (!power_zone) {
+		dev_err(sp->dev, "No SYS domain found\n");
+		return -ENODEV;
+	}
+
+	/* Register other domains as children */
+	for (sd = sp->domains; sd < sp->domains + SPEL_DOMAIN_MAX; sd++) {
+		struct powercap_zone *parent = sp->power_zone;
+
+		if (sd->id == SPEL_DOMAIN_SYS)
+			continue;
+
+		/* SOC is child of SYS, others are children of SOC */
+		if (sd->id != SPEL_DOMAIN_SOC) {
+			for (i = 0; i < SPEL_DOMAIN_MAX; i++) {
+				if (sp->domains[i].id == SPEL_DOMAIN_SOC) {
+					parent = &sp->domains[i].power_zone;
+					break;
+				}
+			}
+		}
+
+		nr_pl = spel_find_nr_power_limit(sd);
+		power_zone = powercap_register_zone(&sd->power_zone,
+						    sp->control_type,
+						    sd->name, parent,
+						    &zone_ops, nr_pl,
+						    &constraint_ops);
+
+		if (IS_ERR(power_zone)) {
+			dev_err(sp->dev, "Failed to register power_zone %s\n",
+				sd->name);
+			ret = PTR_ERR(power_zone);
+			goto err_cleanup;
+		}
+	}
+
+	return 0;
+
+err_cleanup:
+	/* Unregister in reverse order: children first, then SOC, then SYS */
+	for (i = (int)(sd - sp->domains) - 1; i >= 0; i--)
+		powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
+
+	return ret;
+}
+
+static int spel_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct spel_system *sp;
+	struct resource *res;
+	int ret;
+
+	sp = devm_kzalloc(dev, sizeof(*sp), GFP_KERNEL);
+	if (!sp)
+		return -ENOMEM;
+
+	sp->dev = dev;
+
+	/* Map spel domain registers (energy counters) */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nodes");
+	if (!res) {
+		dev_err(dev, "Failed to get nodes resource\n");
+		return -EINVAL;
+	}
+	sp->node_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(sp->node_base))
+		return PTR_ERR(sp->node_base);
+
+	/* Map constraint registers (power limits) */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "constraints");
+	if (!res) {
+		dev_err(dev, "Failed to get constraints resource\n");
+		return -EINVAL;
+	}
+	sp->constraint_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(sp->constraint_base))
+		return PTR_ERR(sp->constraint_base);
+
+	/* Map config registers (units, capabilities) */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
+	if (!res) {
+		dev_err(dev, "Failed to get config resource\n");
+		return -EINVAL;
+	}
+	sp->config_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(sp->config_base))
+		return PTR_ERR(sp->config_base);
+
+	sp->control_type = powercap_register_control_type(NULL, "qcom-spel",
+							  NULL);
+	if (IS_ERR(sp->control_type)) {
+		dev_err(dev, "Failed to register control type\n");
+		return PTR_ERR(sp->control_type);
+	}
+
+	/* Initialize system and domains */
+	ret = spel_init_system(sp, dev);
+	if (ret) {
+		dev_err(dev, "Failed to initialize system\n");
+		goto err_unregister_control;
+	}
+
+	ret = spel_register_powercap(sp);
+	if (ret) {
+		dev_err(dev, "Failed to register powercap zones\n");
+		goto err_unregister_control;
+	}
+
+	platform_set_drvdata(pdev, sp);
+
+	return 0;
+
+err_unregister_control:
+	powercap_unregister_control_type(sp->control_type);
+	return ret;
+}
+
+static void spel_remove(struct platform_device *pdev)
+{
+	struct spel_system *sp = platform_get_drvdata(pdev);
+	int i;
+
+	if (!sp)
+		return;
+
+	/* Unregister in reverse order: children first, then SOC, then SYS */
+	for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
+		powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
+
+	powercap_unregister_control_type(sp->control_type);
+}
+
+static const struct of_device_id spel_of_match[] = {
+	{ .compatible = "qcom,spel" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, spel_of_match);
+
+static struct platform_driver spel_driver = {
+	.probe = spel_probe,
+	.remove = spel_remove,
+	.driver = {
+		.name = "qcom_spel",
+		.of_match_table = spel_of_match,
+	},
+};
+
+module_platform_driver(spel_driver);
+
+MODULE_DESCRIPTION("Qualcomm SPEL Powercap Driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Use-After-Free of `struct powercap_zone` due to `devm_kcalloc` allocation mismatch with sysfs lifecycle.
- [Medium] Phantom power limit constraints are exposed for domains lacking hardware support and register definitions.
- [Medium] Writing the time window register completely overwrites it, clobbering reserved or unmapped bits.
- [Medium] Missing bounds check for time window values causes silent truncation.
--

commit ea2fae1431dc06b5cc4040ca37ee79438a5a76d5
Author: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>

powercap: qcom: Add SPEL powercap driver

This commit adds a new powercap driver for the Qualcomm SoC Power and
Electrical Limits (SPEL) hardware. It provides hardware-based power
monitoring and limiting capabilities for various power domains.

> diff --git a/drivers/powercap/qcom_spel.c b/drivers/powercap/qcom_spel.c
> new file mode 100644
> index 0000000000000..fed5647959a53
> --- /dev/null
> +++ b/drivers/powercap/qcom_spel.c
[ ... ]
> +static int spel_write_pl_data(struct spel_domain *sd, int pl,
> +			      enum pl_ops_type pl_op, unsigned long long value)
> +{
[ ... ]
> +	case PL_LIMIT:
> +		new_val = spel_unit_xlate(sd, POWER_UNIT, value, 1);
> +		if (new_val > POWER_LIMIT_MASK)
> +			return -EINVAL;
> +		reg_val = (reg_val & ~POWER_LIMIT_MASK) | new_val;
[ ... ]
> +	case PL_TIME_WINDOW:
> +		/*
> +		 * Encode time window: upper 7 bits to [22:16], lower 15 bits to [14:0]
> +		 * Time window register is separate from limit register (different offset),
> +		 * so we write only the time window bits without preserving any enable bit.
> +		 */
> +		new_val = spel_unit_xlate(sd, TIME_UNIT, value, 1);

Could this silently truncate the time window if the user provides a value
that exceeds the hardware's 22-bit capacity (0x3FFFFF)? 

It looks like there is no bounds check here, similar to the one for PL_LIMIT
above, so the bitwise operations would just discard the upper bits.

> +		reg_val = (((new_val >> 15) & 0x7F) << 16) |
> +			  (new_val & 0x7FFF);

Does this assignment clobber reserved or unmapped bits in the register?

The original register value is read into reg_val via readl(), but this
directly overwrites it, discarding bit 15 and bits 23-31. Should this
perform a read-modify-write by masking out only TIME_WINDOW_MASK_H and
TIME_WINDOW_MASK_L instead?

> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	writel(reg_val, reg_addr);
> +	return 0;
> +}
[ ... ]
> +static void spel_init_domains(struct spel_system *sp)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < SPEL_DOMAIN_MAX; i++) {
> +		struct spel_domain *sd = &sp->domains[i];
> +
> +		sd->sp = sp;
> +		snprintf(sd->name, SPEL_DOMAIN_NAME_LENGTH, "%s",
> +			 spel_domain_names[i]);
> +		sd->id = i;
> +		sd->status_reg = sp->node_base + domain_offsets[i];
> +
> +		/* PL1 is always supported (required for powercap registration) */
> +		sp->limits[i] = BIT(POWER_LIMIT1);
> +		sd->pl_name[POWER_LIMIT1] = pl_names[POWER_LIMIT1];

Does this expose phantom power limit constraints for domains lacking hardware
support? 

The constraints array only defines register offsets for SPEL_DOMAIN_SYS and
SPEL_DOMAIN_SOC. If POWER_LIMIT1 is unconditionally enabled for all domains
like CL0 and IGPU, any userspace read or write to these constraint attributes
will fall back to get_pl_ops_offset() and return -EOPNOTSUPP since there are
no matching entries.

Since the powercap framework supports zones with 0 constraints, should the
driver rely purely on the hardware capabilities register to discover supported
limits instead?

> +	}
> +}
[ ... ]
> +static int spel_init_system(struct spel_system *sp, struct device *dev)
> +{
> +	int i, ret;
> +
> +	/* Read unit configuration (common for all domains) */
> +	ret = spel_check_unit(sp);
> +	if (ret) {
> +		dev_err(dev, "Failed to read unit config\n");
> +		return ret;
> +	}
> +
> +	sp->domains = devm_kcalloc(dev, SPEL_DOMAIN_MAX,
> +				   sizeof(struct spel_domain), GFP_KERNEL);

Can this lead to a use-after-free if userspace holds a sysfs file open?

The driver embeds a struct powercap_zone inside struct spel_domain, which is
allocated via devm_kcalloc() here.

When the driver is unbound, powercap_unregister_zone() is called, which calls
device_unregister() to drop a reference. If user-space holds a sysfs file
open, the device reference count remains > 0. When remove() returns, devres
will immediately free the memory. Once the file is closed, the final kref_put()
will attempt to invoke the .release callback on the already-freed memory.

Should structures embedding reference-counted devices be dynamically allocated
and freed inside their .release callback?

> +	if (!sp->domains)
> +		return -ENOMEM;
> +
> +	spel_init_domains(sp);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-qcom_spel_driver_upstream-v1-0-75356d1b7f94@oss.qualcomm.com?part=2

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Konrad Dybcio]

On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various power
> domains including System, SoC, CPU clusters, GPU, and various other
> subsystems.
> 
> The driver integrates with the Linux powercap framework, exposing SPEL
> capabilities through powercap sysfs interfaces.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---

[...]

> +/* SPEL register bitmasks */
> +#define ENERGY_STATUS_MASK		0xFFFFFFFF

GENMASK(m, n), across the other defines too, please

Then, you can drop the _OFFSET defines as FIELD_PREP/GET/MODIFY
accessors will derive them from the mask

Please also use lowercase hex, file-wide

[...]

> +/* Constraint configuration */
> +static struct spel_constraint_info constraints[] = {
> +	/* SYS domain constraints */
> +	{ 0x10, 0x70, BIT(0), SPEL_DOMAIN_SYS, POWER_LIMIT1 },
> +	{ 0x14, 0x74, BIT(1), SPEL_DOMAIN_SYS, POWER_LIMIT2 },
> +	{ 0x18, 0x78, BIT(2), SPEL_DOMAIN_SYS, POWER_LIMIT3 },
> +	{ 0x1C, 0x7C, BIT(3), SPEL_DOMAIN_SYS, POWER_LIMIT4 },
> +	/* SOC domain constraints */

"SoC"


> +/* Helper functions */
> +static bool is_pl_valid(struct spel_domain *sd, int pl)
> +{
> +	if (pl < POWER_LIMIT1 || pl >= NR_POWER_LIMITS)
> +		return false;
> +	return sd->pl_name[pl] ? true : false;

return !!sd->pl_name[pl]

[...]

> +static u64 spel_unit_xlate(struct spel_domain *sd, enum unit_type type,
> +			   u64 value, int to_raw)
> +{
> +	struct spel_system *sp = sd->sp;
> +	u64 units = 1;
> +	u64 scale = 1;
> +
> +	switch (type) {
> +	case POWER_UNIT:
> +		units = sp->power_unit;
> +		break;
> +	case ENERGY_UNIT:
> +		scale = ENERGY_UNIT_SCALE;
> +		units = sp->energy_unit;
> +		break;
> +	case TIME_UNIT:
> +		units = sp->time_unit;
> +		break;
> +	default:
> +		return value;

nit: maybe setting units and scale explicitly in each entry could
be better for maintainability, but potayto/potahto


> +static int spel_register_powercap(struct spel_system *sp)
> +{
> +	struct spel_domain *sd;
> +	struct powercap_zone *power_zone = NULL;
> +	int nr_pl, ret, i;
> +
> +	/* Register SYS domain as parent zone */
> +	for (sd = sp->domains; sd < sp->domains + SPEL_DOMAIN_MAX; sd++) {
> +		if (sd->id == SPEL_DOMAIN_SYS) {
> +			nr_pl = spel_find_nr_power_limit(sd);
> +
> +			power_zone = powercap_register_zone(&sd->power_zone,
> +							    sp->control_type, sd->name,
> +					NULL, &zone_ops, nr_pl,
> +					&constraint_ops);
> +			if (IS_ERR(power_zone)) {
> +				dev_err(sp->dev, "Failed to register power zone %s\n",
> +					sd->name);
> +				return PTR_ERR(power_zone);
> +			}
> +			sp->power_zone = power_zone;
> +			break;
> +		}
> +	}
> +
> +	if (!power_zone) {

I believe this is only possible if ARRAY_SIZE(sp->domains) == 0,
but it's not obivous that it's to protect it from that specifically

[...]

> +	/* Map spel domain registers (energy counters) */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nodes");
> +	if (!res) {
> +		dev_err(dev, "Failed to get nodes resource\n");
> +		return -EINVAL;
> +	}
> +	sp->node_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(sp->node_base))
> +		return PTR_ERR(sp->node_base);

devm_platform_get_and_ioremap_resource()

[...]

> +static void spel_remove(struct platform_device *pdev)
> +{
> +	struct spel_system *sp = platform_get_drvdata(pdev);
> +	int i;
> +
> +	if (!sp)
> +		return;
> +
> +	/* Unregister in reverse order: children first, then SOC, then SYS */
> +	for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
> +		powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);

Could you try adding a devm_ variant of these register functions?

[...]
> +static const struct of_device_id spel_of_match[] = {
> +	{ .compatible = "qcom,spel" },

The compatible must contain a SoC name

Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,


On 5/21/2026 4:46 PM, Konrad Dybcio wrote:
> On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>> based power monitoring and limiting capabilities for various power
>> domains including System, SoC, CPU clusters, GPU, and various other
>> subsystems.
>>
>> The driver integrates with the Linux powercap framework, exposing SPEL
>> capabilities through powercap sysfs interfaces.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +/* SPEL register bitmasks */
>> +#define ENERGY_STATUS_MASK		0xFFFFFFFF
> 
> GENMASK(m, n), across the other defines too, please
> 
> Then, you can drop the _OFFSET defines as FIELD_PREP/GET/MODIFY
> accessors will derive them from the mask
> 
> Please also use lowercase hex, file-wide

ACK

> 
> [...]
> 
>> +/* Constraint configuration */
>> +static struct spel_constraint_info constraints[] = {
>> +	/* SYS domain constraints */
>> +	{ 0x10, 0x70, BIT(0), SPEL_DOMAIN_SYS, POWER_LIMIT1 },
>> +	{ 0x14, 0x74, BIT(1), SPEL_DOMAIN_SYS, POWER_LIMIT2 },
>> +	{ 0x18, 0x78, BIT(2), SPEL_DOMAIN_SYS, POWER_LIMIT3 },
>> +	{ 0x1C, 0x7C, BIT(3), SPEL_DOMAIN_SYS, POWER_LIMIT4 },
>> +	/* SOC domain constraints */
> 
> "SoC"

ACK

> 
> 
>> +/* Helper functions */
>> +static bool is_pl_valid(struct spel_domain *sd, int pl)
>> +{
>> +	if (pl < POWER_LIMIT1 || pl >= NR_POWER_LIMITS)
>> +		return false;
>> +	return sd->pl_name[pl] ? true : false;
> 
> return !!sd->pl_name[pl]

ACK

> 
> [...]
> 
>> +static u64 spel_unit_xlate(struct spel_domain *sd, enum unit_type type,
>> +			   u64 value, int to_raw)
>> +{
>> +	struct spel_system *sp = sd->sp;
>> +	u64 units = 1;
>> +	u64 scale = 1;
>> +
>> +	switch (type) {
>> +	case POWER_UNIT:
>> +		units = sp->power_unit;
>> +		break;
>> +	case ENERGY_UNIT:
>> +		scale = ENERGY_UNIT_SCALE;
>> +		units = sp->energy_unit;
>> +		break;
>> +	case TIME_UNIT:
>> +		units = sp->time_unit;
>> +		break;
>> +	default:
>> +		return value;
> 
> nit: maybe setting units and scale explicitly in each entry could
> be better for maintainability, but potayto/potahto

ACK

> 
>> +static int spel_register_powercap(struct spel_system *sp)
>> +{
>> +	struct spel_domain *sd;
>> +	struct powercap_zone *power_zone = NULL;
>> +	int nr_pl, ret, i;
>> +
>> +	/* Register SYS domain as parent zone */
>> +	for (sd = sp->domains; sd < sp->domains + SPEL_DOMAIN_MAX; sd++) {
>> +		if (sd->id == SPEL_DOMAIN_SYS) {
>> +			nr_pl = spel_find_nr_power_limit(sd);
>> +
>> +			power_zone = powercap_register_zone(&sd->power_zone,
>> +							    sp->control_type, sd->name,
>> +					NULL, &zone_ops, nr_pl,
>> +					&constraint_ops);
>> +			if (IS_ERR(power_zone)) {
>> +				dev_err(sp->dev, "Failed to register power zone %s\n",
>> +					sd->name);
>> +				return PTR_ERR(power_zone);
>> +			}
>> +			sp->power_zone = power_zone;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!power_zone) {
> 
> I believe this is only possible if ARRAY_SIZE(sp->domains) == 0,
> but it's not obivous that it's to protect it from that specifically

It will also catch a case where domains defined without root domain 
SPEL_DOMAIN_SYS
> 
> [...]
> 
>> +	/* Map spel domain registers (energy counters) */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nodes");
>> +	if (!res) {
>> +		dev_err(dev, "Failed to get nodes resource\n");
>> +		return -EINVAL;
>> +	}
>> +	sp->node_base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(sp->node_base))
>> +		return PTR_ERR(sp->node_base);
> 
> devm_platform_get_and_ioremap_resource()

ACK

> 
> [...]
> 
>> +static void spel_remove(struct platform_device *pdev)
>> +{
>> +	struct spel_system *sp = platform_get_drvdata(pdev);
>> +	int i;
>> +
>> +	if (!sp)
>> +		return;
>> +
>> +	/* Unregister in reverse order: children first, then SOC, then SYS */
>> +	for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
>> +		powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
> 
> Could you try adding a devm_ variant of these register functions?

Powercap framework doesn't support any devm_* API, you meant add this 
support in framework in this series ?

> 
> [...]
>> +static const struct of_device_id spel_of_match[] = {
>> +	{ .compatible = "qcom,spel" },
> 
> The compatible must contain a SoC name

ACK

Thanks,
Manaf
> 
> Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Konrad Dybcio]

On 6/9/26 3:23 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Konrad,
> 
> 
> On 5/21/2026 4:46 PM, Konrad Dybcio wrote:
>> On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>>> based power monitoring and limiting capabilities for various power
>>> domains including System, SoC, CPU clusters, GPU, and various other
>>> subsystems.
>>>
>>> The driver integrates with the Linux powercap framework, exposing SPEL
>>> capabilities through powercap sysfs interfaces.
>>>
>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>> ---

[...]

>>> +static void spel_remove(struct platform_device *pdev)
>>> +{
>>> +    struct spel_system *sp = platform_get_drvdata(pdev);
>>> +    int i;
>>> +
>>> +    if (!sp)
>>> +        return;
>>> +
>>> +    /* Unregister in reverse order: children first, then SOC, then SYS */
>>> +    for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
>>> +        powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
>>
>> Could you try adding a devm_ variant of these register functions?
> 
> Powercap framework doesn't support any devm_* API, you meant add this support in framework in this series ?

Yes, this should be fairly trivial - look at e.g.

drivers/regulator/devres.c : devm_of_regulator_get

Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Daniel Lezcano]

Hi Konrad,

Le 09/06/2026 à 15:31, Konrad Dybcio a écrit :
> On 6/9/26 3:23 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Konrad,
>>
>>
>> On 5/21/2026 4:46 PM, Konrad Dybcio wrote:
>>> On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>>>> based power monitoring and limiting capabilities for various power
>>>> domains including System, SoC, CPU clusters, GPU, and various other
>>>> subsystems.
>>>>
>>>> The driver integrates with the Linux powercap framework, exposing SPEL
>>>> capabilities through powercap sysfs interfaces.
>>>>
>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>> ---
> 
> [...]
> 
>>>> +static void spel_remove(struct platform_device *pdev)
>>>> +{
>>>> +    struct spel_system *sp = platform_get_drvdata(pdev);
>>>> +    int i;
>>>> +
>>>> +    if (!sp)
>>>> +        return;
>>>> +
>>>> +    /* Unregister in reverse order: children first, then SOC, then SYS */
>>>> +    for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
>>>> +        powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
>>>
>>> Could you try adding a devm_ variant of these register functions?
>>
>> Powercap framework doesn't support any devm_* API, you meant add this support in framework in this series ?
> 
> Yes, this should be fairly trivial - look at e.g.
> 
> drivers/regulator/devres.c : devm_of_regulator_get

Given how structured is the powercap framework, I'm not sure it is so 
trivial. May be it is, but I doubt.

I don't think it is worth to investigate adding devm_ variant now (for a 
few lines saved) and diverge from the objective of this series

   -- Daniel

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Konrad Dybcio]

On 6/15/26 2:07 PM, Daniel Lezcano wrote:
> Hi Konrad,
> 
> Le 09/06/2026 à 15:31, Konrad Dybcio a écrit :
>> On 6/9/26 3:23 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> Hi Konrad,
>>>
>>>
>>> On 5/21/2026 4:46 PM, Konrad Dybcio wrote:
>>>> On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>>>>> based power monitoring and limiting capabilities for various power
>>>>> domains including System, SoC, CPU clusters, GPU, and various other
>>>>> subsystems.
>>>>>
>>>>> The driver integrates with the Linux powercap framework, exposing SPEL
>>>>> capabilities through powercap sysfs interfaces.
>>>>>
>>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>>> ---
>>
>> [...]
>>
>>>>> +static void spel_remove(struct platform_device *pdev)
>>>>> +{
>>>>> +    struct spel_system *sp = platform_get_drvdata(pdev);
>>>>> +    int i;
>>>>> +
>>>>> +    if (!sp)
>>>>> +        return;
>>>>> +
>>>>> +    /* Unregister in reverse order: children first, then SOC, then SYS */
>>>>> +    for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
>>>>> +        powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
>>>>
>>>> Could you try adding a devm_ variant of these register functions?
>>>
>>> Powercap framework doesn't support any devm_* API, you meant add this support in framework in this series ?
>>
>> Yes, this should be fairly trivial - look at e.g.
>>
>> drivers/regulator/devres.c : devm_of_regulator_get
> 
> Given how structured is the powercap framework, I'm not sure it is so trivial. May be it is, but I doubt.
> 
> I don't think it is worth to investigate adding devm_ variant now (for a few lines saved) and diverge from the objective of this series

Might as well be, looks like the powercap zones are "namespaced" inside
powercap controllers, so it's not that trivial

Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Konrad Dybcio]

On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various power
> domains including System, SoC, CPU clusters, GPU, and various other
> subsystems.
> 
> The driver integrates with the Linux powercap framework, exposing SPEL
> capabilities through powercap sysfs interfaces.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---

[...]

> +/* Constraint configuration */
> +static struct spel_constraint_info constraints[] = {
> +	/* SYS domain constraints */
> +	{ 0x10, 0x70, BIT(0), SPEL_DOMAIN_SYS, POWER_LIMIT1 },
> +	{ 0x14, 0x74, BIT(1), SPEL_DOMAIN_SYS, POWER_LIMIT2 },
> +	{ 0x18, 0x78, BIT(2), SPEL_DOMAIN_SYS, POWER_LIMIT3 },
> +	{ 0x1C, 0x7C, BIT(3), SPEL_DOMAIN_SYS, POWER_LIMIT4 },
> +	/* SOC domain constraints */
> +	{ 0x00, 0x60, BIT(4), SPEL_DOMAIN_SOC, POWER_LIMIT1 },
> +	{ 0x04, 0x64, BIT(5), SPEL_DOMAIN_SOC, POWER_LIMIT2 },
> +	{ 0x08, 0x68, BIT(6), SPEL_DOMAIN_SOC, POWER_LIMIT3 },
> +	{ 0x0C, 0x6C, BIT(7), SPEL_DOMAIN_SOC, POWER_LIMIT4 },

Lowercase hex, please

Can this data be const?

Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,


On 5/21/2026 4:47 PM, Konrad Dybcio wrote:
> On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>> based power monitoring and limiting capabilities for various power
>> domains including System, SoC, CPU clusters, GPU, and various other
>> subsystems.
>>
>> The driver integrates with the Linux powercap framework, exposing SPEL
>> capabilities through powercap sysfs interfaces.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +/* Constraint configuration */
>> +static struct spel_constraint_info constraints[] = {
>> +	/* SYS domain constraints */
>> +	{ 0x10, 0x70, BIT(0), SPEL_DOMAIN_SYS, POWER_LIMIT1 },
>> +	{ 0x14, 0x74, BIT(1), SPEL_DOMAIN_SYS, POWER_LIMIT2 },
>> +	{ 0x18, 0x78, BIT(2), SPEL_DOMAIN_SYS, POWER_LIMIT3 },
>> +	{ 0x1C, 0x7C, BIT(3), SPEL_DOMAIN_SYS, POWER_LIMIT4 },
>> +	/* SOC domain constraints */
>> +	{ 0x00, 0x60, BIT(4), SPEL_DOMAIN_SOC, POWER_LIMIT1 },
>> +	{ 0x04, 0x64, BIT(5), SPEL_DOMAIN_SOC, POWER_LIMIT2 },
>> +	{ 0x08, 0x68, BIT(6), SPEL_DOMAIN_SOC, POWER_LIMIT3 },
>> +	{ 0x0C, 0x6C, BIT(7), SPEL_DOMAIN_SOC, POWER_LIMIT4 },
> 
> Lowercase hex, please
> 
> Can this data be const?

ACK

Thanks,
Manaf

> 
> Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Konrad Dybcio]

On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various power
> domains including System, SoC, CPU clusters, GPU, and various other
> subsystems.
> 
> The driver integrates with the Linux powercap framework, exposing SPEL
> capabilities through powercap sysfs interfaces.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---

[...]

> +static const char *const spel_domain_names[] = {
> +	"sys", "soc", "cl0", "cl1", "cl2", "igpu", "dgpu", "nsp",
> +	"mmcx", "infra", "dram", "mdm", "wlan", "usb1", "usb2", "usb3",
> +};

If the domain data is not going to change for future platforms (but
e.g. only the offsets will), which I doubt, please use designated
initializers

Otherwise, please coalesce the names and offsets in a struct

Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,


On 5/21/2026 4:49 PM, Konrad Dybcio wrote:
> On 5/19/26 12:49 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>> based power monitoring and limiting capabilities for various power
>> domains including System, SoC, CPU clusters, GPU, and various other
>> subsystems.
>>
>> The driver integrates with the Linux powercap framework, exposing SPEL
>> capabilities through powercap sysfs interfaces.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static const char *const spel_domain_names[] = {
>> +	"sys", "soc", "cl0", "cl1", "cl2", "igpu", "dgpu", "nsp",
>> +	"mmcx", "infra", "dram", "mdm", "wlan", "usb1", "usb2", "usb3",
>> +};
> 
> If the domain data is not going to change for future platforms (but
> e.g. only the offsets will), which I doubt, please use designated
> initializers

It can change platform to platform.

> 
> Otherwise, please coalesce the names and offsets in a struct

ACK

Thanks,
Manaf

> 
> Konrad

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Daniel Lezcano]

On 5/19/26 12:49, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various power
> domains including System, SoC, CPU clusters, GPU, and various other
> subsystems.
> 
> The driver integrates with the Linux powercap framework, exposing SPEL
> capabilities through powercap sysfs interfaces.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>   MAINTAINERS                  |   1 +
>   drivers/powercap/Kconfig     |  13 +
>   drivers/powercap/Makefile    |   1 +
>   drivers/powercap/qcom_spel.c | 787 +++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 802 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c63f147e8c54..5c7542754ab6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22238,6 +22238,7 @@ M:	Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>   L:	linux-arm-msm@vger.kernel.org
>   S:	Maintained
>   F:	Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
> +F:	drivers/powercap/qcom_spel.c
>   
>   QUALCOMM PPE DRIVER
>   M:	Luo Jie <quic_luoj@quicinc.com>
> diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
> index 03c4c796d993..e3a47c653499 100644
> --- a/drivers/powercap/Kconfig
> +++ b/drivers/powercap/Kconfig
> @@ -93,4 +93,17 @@ config DTPM_DEVFREQ
>   	help
>   	  This enables support for device power limitation based on
>   	  energy model.
> +
> +config QCOM_SPEL
> +	tristate "Qualcomm SPEL Powercap driver"
> +	depends on ARM64 || COMPILE_TEST
> +	help
> +	  This enables support for the Qualcomm SoC Power and Electrical
> +	  Limits (SPEL) hardware, which allows power limits to be
> +	  enforced and monitored on Qualcomm SoCs.
> +
> +	  SPEL provides energy monitoring and power capping for multiple
> +	  domains including system, SoC, CPU clusters, GPU, and various
> +	  other subsystems.
> +
>   endif
> diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile
> index 5ab0dce565b9..8235fb9d3df6 100644
> --- a/drivers/powercap/Makefile
> +++ b/drivers/powercap/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_INTEL_RAPL) += intel_rapl_msr.o
>   obj-$(CONFIG_INTEL_RAPL_TPMI) += intel_rapl_tpmi.o
>   obj-$(CONFIG_IDLE_INJECT) += idle_inject.o
>   obj-$(CONFIG_ARM_SCMI_POWERCAP) += arm_scmi_powercap.o
> +obj-$(CONFIG_QCOM_SPEL) += qcom_spel.o
> diff --git a/drivers/powercap/qcom_spel.c b/drivers/powercap/qcom_spel.c
> new file mode 100644
> index 000000000000..fed5647959a5
> --- /dev/null
> +++ b/drivers/powercap/qcom_spel.c
> @@ -0,0 +1,787 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Qualcomm SPEL (SoC Power and Electrical Limits) Driver
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/bitmap.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/powercap.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +/* SPEL register bitmasks */
> +#define ENERGY_STATUS_MASK		0xFFFFFFFF
> +
> +#define POWER_LIMIT_MASK		0x00007FFF
> +#define POWER_LIMIT_ENABLE		BIT(31)
> +
> +#define TIME_WINDOW_MASK_L		0x00007FFF	/* bits [14:0] */
> +#define TIME_WINDOW_MASK_H		0x007F0000	/* bits [22:16] */
> +
> +#define ENERGY_UNIT_OFFSET		16
> +#define ENERGY_UNIT_MASK		0xF0000
> +
> +#define TIME_UNIT_OFFSET		8
> +#define TIME_UNIT_MASK			0xF00
> +
> +#define POWER_UNIT_OFFSET		0
> +#define POWER_UNIT_MASK			0x7
> +
> +#define LIMITS_CAPABILITY_OFFSET	0x20
> +#define ENERGY_RPT_UNIT_OFFSET		0x04
> +
> +#define ENERGY_UNIT_SCALE		1000
> +
> +#define SPEL_DOMAIN_NAME_LENGTH		16
> +
> +/* Domain types */
> +enum spel_domain_type {
> +	SPEL_DOMAIN_SYS,
> +	SPEL_DOMAIN_SOC,
> +	SPEL_DOMAIN_CL0,
> +	SPEL_DOMAIN_CL1,
> +	SPEL_DOMAIN_CL2,
> +	SPEL_DOMAIN_IGPU,
> +	SPEL_DOMAIN_DGPU,
> +	SPEL_DOMAIN_NSP,
> +	SPEL_DOMAIN_MMCX,
> +	SPEL_DOMAIN_INFRA,
> +	SPEL_DOMAIN_DRAM,
> +	SPEL_DOMAIN_MDM,
> +	SPEL_DOMAIN_WLAN,
> +	SPEL_DOMAIN_USB1,
> +	SPEL_DOMAIN_USB2,
> +	SPEL_DOMAIN_USB3,
> +	SPEL_DOMAIN_MAX,
> +};
> +
> +/* Power limit IDs */
> +enum spel_power_limit_id {
> +	POWER_LIMIT1,
> +	POWER_LIMIT2,
> +	POWER_LIMIT3,
> +	POWER_LIMIT4,
> +	NR_POWER_LIMITS,
> +};
> +
> +/* Unit types for conversion */
> +enum unit_type {
> +	POWER_UNIT,
> +	ENERGY_UNIT,
> +	TIME_UNIT,
> +};
> +
> +/* Power limit operation types */
> +enum pl_ops_type {
> +	PL_LIMIT,
> +	PL_TIME_WINDOW,
> +};
> +
> +static const char *pl_names[NR_POWER_LIMITS] = {
> +	[POWER_LIMIT1] = "pl1",
> +	[POWER_LIMIT2] = "pl2",
> +	[POWER_LIMIT3] = "pl3",
> +	[POWER_LIMIT4] = "pl4",
> +};

Do you want to use NR_POWER_LIMITS instead of ARRAY_SIZE() ?

Please unify the naming NR_POWER_LIMITS -> POWER_LIMITS_MAX

> +static const char *const spel_domain_names[] = {
> +	"sys", "soc", "cl0", "cl1", "cl2", "igpu", "dgpu", "nsp",
> +	"mmcx", "infra", "dram", "mdm", "wlan", "usb1", "usb2", "usb3",
> +};
> +
> +/* Domain register offsets in node base */
> +static const u32 domain_offsets[SPEL_DOMAIN_MAX] = {
> +	[SPEL_DOMAIN_SYS]	= 0x40,
> +	[SPEL_DOMAIN_SOC]	= 0x00,
> +	[SPEL_DOMAIN_CL0]	= 0x5C,
> +	[SPEL_DOMAIN_CL1]	= 0x60,
> +	[SPEL_DOMAIN_CL2]	= 0x64,
> +	[SPEL_DOMAIN_IGPU]	= 0x08,
> +	[SPEL_DOMAIN_DGPU]	= 0x44,
> +	[SPEL_DOMAIN_NSP]	= 0x0C,
> +	[SPEL_DOMAIN_MMCX]	= 0x10,
> +	[SPEL_DOMAIN_INFRA]	= 0x18,
> +	[SPEL_DOMAIN_DRAM]	= 0x1C,
> +	[SPEL_DOMAIN_MDM]	= 0x48,
> +	[SPEL_DOMAIN_WLAN]	= 0x4C,
> +	[SPEL_DOMAIN_USB1]	= 0x50,
> +	[SPEL_DOMAIN_USB2]	= 0x54,
> +	[SPEL_DOMAIN_USB3]	= 0x58,
> +};

Same comment

> +/**
> + * struct spel_constraint_info - Power limit constraint information
> + * @limit_offset:	Register offset for power limit value
> + * @time_window_offset:	Register offset for time window
> + * @supported_mask:	Bit mask in capability register

Where is 'supported_mask' initialized?

> + * @domain_id:		Domain this constraint applies to
> + * @pl_id:		Power limit ID (PL1, PL2, etc.)

[ ... ]

> +
> +/**
> + * struct spel_domain - SPEL power domain
> + * @power_zone:		Powercap zone
> + * @lock:		Mutex protecting register access
> + * @sp:			Parent system
> + * @status_reg:		Energy counter register
> + * @pl_name:		Power limit names
> + * @name:		Domain name
> + * @id:			Domain type ID
> + */
> +struct spel_domain {

[ ... ]

> +	struct spel_system *sp;

[ ... ]

> +struct spel_system {
> +	struct spel_domain *domains;

[ ... ]

> +};

There is a cyclic dependency between struct spel_system <-> struct 
spel_domain. Could it be solved ?

> +#define power_zone_to_spel_domain(_zone) \
> +	container_of(_zone, struct spel_domain, power_zone)
> +
> +/* Helper functions */
> +static bool is_pl_valid(struct spel_domain *sd, int pl)
> +{
> +	if (pl < POWER_LIMIT1 || pl >= NR_POWER_LIMITS)
> +		return false;

The call to this function is strange. It is like you don't trust your 
own code.

> +	return sd->pl_name[pl] ? true : false;
> +}
> +

[ ... ]

> +
> +	switch (pl_op) {
> +	case PL_LIMIT:
> +		value &= POWER_LIMIT_MASK;
> +		if (xlate)
> +			*data = spel_unit_xlate(sd, POWER_UNIT, value, 0);
> +		else
> +			*data = value;
> +		break;
> +	case PL_TIME_WINDOW:
> +		/* Decode time window: bits [22:16] are upper 7 bits, [14:0] are lower 15 bits */
> +		value = ((value & TIME_WINDOW_MASK_H) >> 16 << 15) |
> +			(value & TIME_WINDOW_MASK_L);

[ ... ]

> +		reg_val = (reg_val & ~POWER_LIMIT_MASK) | new_val;
> +

[ ... ]

> +		if (new_val == 0)
> +			reg_val &= ~POWER_LIMIT_ENABLE;
> +		else
> +			reg_val |= POWER_LIMIT_ENABLE;
> +		break;

[ ... ]

> +	case PL_TIME_WINDOW:
> +		/*
> +		 * Encode time window: upper 7 bits to [22:16], lower 15 bits to [14:0]
> +		 * Time window register is separate from limit register (different offset),
> +		 * so we write only the time window bits without preserving any enable bit.
> +		 */
> +		new_val = spel_unit_xlate(sd, TIME_UNIT, value, 1);
> +		reg_val = (((new_val >> 15) & 0x7F) << 16) |
> +			  (new_val & 0x7FFF);


The trend today is to use the FIELD_* macros for bits ops

> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	writel(reg_val, reg_addr);
> +	return 0;
> +}
> +
> +/* Powercap zone operations */
> +static int spel_get_energy_counter(struct powercap_zone *power_zone, u64 *energy_raw)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> +	u64 value;
> +
> +	value = readl(sd->status_reg);

nit: seems an line in between would help for the readability

> +	*energy_raw = spel_unit_xlate(sd, ENERGY_UNIT, value, 0);
> +
> +	return 0;
> +}
> +
> +static int spel_get_max_energy_counter(struct powercap_zone *pcd_dev, u64 *energy)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(pcd_dev);
> +
> +	*energy = spel_unit_xlate(sd, ENERGY_UNIT, ENERGY_STATUS_MASK, 0);

nit: missing line

> +	return 0;
> +}
> +
> +static int spel_release_zone(struct powercap_zone *power_zone)
> +{
> +	return 0;
> +}
> +
> +static int spel_find_nr_power_limit(struct spel_domain *sd)
> +{
> +	int i, nr_pl = 0;
> +
> +	for (i = 0; i < NR_POWER_LIMITS; i++) {
> +		if (is_pl_valid(sd, i))
> +			nr_pl++;
> +	}
> +
> +	return nr_pl;
> +}
> +
> +static const struct powercap_zone_ops zone_ops = {
> +	.get_energy_uj = spel_get_energy_counter,
> +	.get_max_energy_range_uj = spel_get_max_energy_counter,
> +	.release = spel_release_zone,
> +};
> +
> +/* Constraint operations */
> +static int spel_constraint_to_pl(struct spel_domain *sd, int cid)
> +{
> +	int i, j;

'j' name is misleading because it is usually used for nested 'for' blocks

> +	for (i = POWER_LIMIT1, j = 0; i < NR_POWER_LIMITS; i++) {

Do not rely on POWER_LIMIT1 because if someday it is moved in the enum, 
all the code assuming it is zero will be broken

> +		if (is_pl_valid(sd, i) && j++ == cid)
 > +			return i;> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int spel_set_power_limit(struct powercap_zone *power_zone, int cid,
> +				u64 power_limit)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> +	int id;
> +
> +	id = spel_constraint_to_pl(sd, cid);
> +	if (id < 0)
> +		return id;
> +
> +	return spel_write_pl_data(sd, id, PL_LIMIT, power_limit);
> +}
> +
> +static int spel_get_power_limit(struct powercap_zone *power_zone, int cid,
> +				u64 *data)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> +	u64 val;
> +	int ret, id;
> +
> +	id = spel_constraint_to_pl(sd, cid);
> +	if (id < 0)
> +		return id;
> +
> +	ret = spel_read_pl_data(sd, id, PL_LIMIT, true, &val);
> +	if (!ret)
> +		*data = val;
> +
> +	return ret;
> +}
> +
> +static int spel_set_time_window(struct powercap_zone *power_zone, int cid,
> +				u64 window)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> +	int id;
> +
> +	id = spel_constraint_to_pl(sd, cid);
> +	if (id < 0)
> +		return id;
> +
> +	return spel_write_pl_data(sd, id, PL_TIME_WINDOW, window);
> +}
> +
> +static int spel_get_time_window(struct powercap_zone *power_zone, int cid,
> +				u64 *data)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> +	u64 val;
> +	int ret, id;
> +
> +	id = spel_constraint_to_pl(sd, cid);
> +	if (id < 0)
> +		return id;
> +
> +	ret = spel_read_pl_data(sd, id, PL_TIME_WINDOW, true, &val);
> +	if (!ret)
> +		*data = val;
> +
> +	return ret;
> +}
> +
> +static const char *spel_get_constraint_name(struct powercap_zone *power_zone,
> +					    int cid)
> +{
> +	struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> +	int id;
> +
> +	id = spel_constraint_to_pl(sd, cid);
> +	if (id >= 0)
> +		return sd->pl_name[id];
> +
> +	return NULL;
> +}
> +
> +static const struct powercap_zone_constraint_ops constraint_ops = {
> +	.set_power_limit_uw = spel_set_power_limit,
> +	.get_power_limit_uw = spel_get_power_limit,
> +	.set_time_window_us = spel_set_time_window,
> +	.get_time_window_us = spel_get_time_window,
> +	.get_name = spel_get_constraint_name,
> +};
> +
> +static void spel_init_domains(struct spel_system *sp)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < SPEL_DOMAIN_MAX; i++) {
> +		struct spel_domain *sd = &sp->domains[i];
> +
> +		sd->sp = sp;
> +		snprintf(sd->name, SPEL_DOMAIN_NAME_LENGTH, "%s",
> +			 spel_domain_names[i]);
> +		sd->id = i;
> +		sd->status_reg = sp->node_base + domain_offsets[i];
> +
> +		/* PL1 is always supported (required for powercap registration) */
> +		sp->limits[i] = BIT(POWER_LIMIT1);
> +		sd->pl_name[POWER_LIMIT1] = pl_names[POWER_LIMIT1];
> +	}
> +}
> +
> +static int spel_check_unit(struct spel_system *sp)
> +{
> +	u32 value, shift;
> +
> +	/* Read power_unit and time_unit from offset 0x0 */
> +	value = readl(sp->config_base);
> +
> +	/*
> +	 * Unit calculation: 1 / (2^shift)
> +	 * Masks limit: TIME_UNIT (4 bits, max 15), POWER_UNIT (3 bits, max 7).
> +	 */
> +	shift = (value & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
> +	sp->power_unit = 1000000 / (1 << shift);
> +
> +	shift = (value & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
> +	sp->time_unit = 1000000 / (1 << shift);
> +
> +	/* Read energy_unit from ENERGY_RPT_UNIT_OFFSET */
> +	value = readl(sp->config_base + ENERGY_RPT_UNIT_OFFSET);
> +
> +	/*
> +	 * Unit calculation: 1 / (2^shift)
> +	 * Masks limit: ENERGY_UNIT (4 bits, max 15).
> +	 */
> +	shift = (value & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
> +	sp->energy_unit = ENERGY_UNIT_SCALE * 1000000 / (1 << shift);
> +
> +	dev_dbg(sp->dev, "Units: energy=%dnJ, time=%dus, power=%duW\n",
> +		sp->energy_unit, sp->time_unit, sp->power_unit);
> +
> +	return 0;
> +}
> +
> +static void spel_detect_powerlimit(struct spel_domain *sd)
> +{
> +	struct spel_system *sp = sd->sp;
> +	u32 capabilities;
> +	int i, j;
> +
> +	capabilities = readl(sp->config_base + LIMITS_CAPABILITY_OFFSET);
> +
> +	/* Detect power limits from hardware capabilities */
> +	for (i = POWER_LIMIT2; i < NR_POWER_LIMITS; i++) {

For my understanding, why is it starting at POWER_LIMIT2 ?

> +		for (j = 0; j < ARRAY_SIZE(constraints); j++) {
> +			struct spel_constraint_info *ci = &constraints[j];
> +
> +			if (ci->domain_id == sd->id && ci->pl_id == i) {
> +				if (capabilities & ci->supported_mask) {
> +					sp->limits[sd->id] |= BIT(i);
> +					sd->pl_name[i] = pl_names[i];

So, that explains the is_pl_valid()

Please do not use duplicated array with non-NULL pointer checks

Or sd->pl_name[] contains all the *valids* power limits, so its size is 
different than (or equal to) pl_names. Or it is a fixed array with a 
structure containing a flag telling if it is enabled or not.

No need to duplicate the array

> +				}
> +				break;
> +			}
> +		}
> +	}
> +}
> +

[ ... ]

> +static void spel_remove(struct platform_device *pdev)
> +{
> +	struct spel_system *sp = platform_get_drvdata(pdev);
> +	int i;
> +
> +	if (!sp)
> +		return;

Why test if the value is correct? There is no reason it changed after 
'probe' was successful

> +
> +	/* Unregister in reverse order: children first, then SOC, then SYS */
> +	for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
> +		powercap_unregister_zone(sp->control_type, &sp->domains[i].power_zone);
> +
> +	powercap_unregister_control_type(sp->control_type);
> +}
> +
> +static const struct of_device_id spel_of_match[] = {
> +	{ .compatible = "qcom,spel" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, spel_of_match);
> +
> +static struct platform_driver spel_driver = {
> +	.probe = spel_probe,
> +	.remove = spel_remove,
> +	.driver = {
> +		.name = "qcom_spel",
> +		.of_match_table = spel_of_match,
> +	},
> +};
> +
> +module_platform_driver(spel_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm SPEL Powercap Driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 2/3] powercap: qcom: Add SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

Hi Daniel,


On 5/27/2026 12:06 AM, Daniel Lezcano wrote:
> On 5/19/26 12:49, Manaf Meethalavalappu Pallikunhi wrote:
>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>> based power monitoring and limiting capabilities for various power
>> domains including System, SoC, CPU clusters, GPU, and various other
>> subsystems.
>>
>> The driver integrates with the Linux powercap framework, exposing SPEL
>> capabilities through powercap sysfs interfaces.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi 
>> <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   MAINTAINERS                  |   1 +
>>   drivers/powercap/Kconfig     |  13 +
>>   drivers/powercap/Makefile    |   1 +
>>   drivers/powercap/qcom_spel.c | 787 +++++++++++++++++++++++++++++++++ 
>> ++++++++++
>>   4 files changed, 802 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c63f147e8c54..5c7542754ab6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -22238,6 +22238,7 @@ M:    Manaf Meethalavalappu Pallikunhi 
>> <manaf.pallikunhi@oss.qualcomm.com>
>>   L:    linux-arm-msm@vger.kernel.org
>>   S:    Maintained
>>   F:    Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
>> +F:    drivers/powercap/qcom_spel.c
>>   QUALCOMM PPE DRIVER
>>   M:    Luo Jie <quic_luoj@quicinc.com>
>> diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
>> index 03c4c796d993..e3a47c653499 100644
>> --- a/drivers/powercap/Kconfig
>> +++ b/drivers/powercap/Kconfig
>> @@ -93,4 +93,17 @@ config DTPM_DEVFREQ
>>       help
>>         This enables support for device power limitation based on
>>         energy model.
>> +
>> +config QCOM_SPEL
>> +    tristate "Qualcomm SPEL Powercap driver"
>> +    depends on ARM64 || COMPILE_TEST
>> +    help
>> +      This enables support for the Qualcomm SoC Power and Electrical
>> +      Limits (SPEL) hardware, which allows power limits to be
>> +      enforced and monitored on Qualcomm SoCs.
>> +
>> +      SPEL provides energy monitoring and power capping for multiple
>> +      domains including system, SoC, CPU clusters, GPU, and various
>> +      other subsystems.
>> +
>>   endif
>> diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile
>> index 5ab0dce565b9..8235fb9d3df6 100644
>> --- a/drivers/powercap/Makefile
>> +++ b/drivers/powercap/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_INTEL_RAPL) += intel_rapl_msr.o
>>   obj-$(CONFIG_INTEL_RAPL_TPMI) += intel_rapl_tpmi.o
>>   obj-$(CONFIG_IDLE_INJECT) += idle_inject.o
>>   obj-$(CONFIG_ARM_SCMI_POWERCAP) += arm_scmi_powercap.o
>> +obj-$(CONFIG_QCOM_SPEL) += qcom_spel.o
>> diff --git a/drivers/powercap/qcom_spel.c b/drivers/powercap/qcom_spel.c
>> new file mode 100644
>> index 000000000000..fed5647959a5
>> --- /dev/null
>> +++ b/drivers/powercap/qcom_spel.c
>> @@ -0,0 +1,787 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Qualcomm SPEL (SoC Power and Electrical Limits) Driver
>> + *
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/bitmap.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/powercap.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +
>> +/* SPEL register bitmasks */
>> +#define ENERGY_STATUS_MASK        0xFFFFFFFF
>> +
>> +#define POWER_LIMIT_MASK        0x00007FFF
>> +#define POWER_LIMIT_ENABLE        BIT(31)
>> +
>> +#define TIME_WINDOW_MASK_L        0x00007FFF    /* bits [14:0] */
>> +#define TIME_WINDOW_MASK_H        0x007F0000    /* bits [22:16] */
>> +
>> +#define ENERGY_UNIT_OFFSET        16
>> +#define ENERGY_UNIT_MASK        0xF0000
>> +
>> +#define TIME_UNIT_OFFSET        8
>> +#define TIME_UNIT_MASK            0xF00
>> +
>> +#define POWER_UNIT_OFFSET        0
>> +#define POWER_UNIT_MASK            0x7
>> +
>> +#define LIMITS_CAPABILITY_OFFSET    0x20
>> +#define ENERGY_RPT_UNIT_OFFSET        0x04
>> +
>> +#define ENERGY_UNIT_SCALE        1000
>> +
>> +#define SPEL_DOMAIN_NAME_LENGTH        16
>> +
>> +/* Domain types */
>> +enum spel_domain_type {
>> +    SPEL_DOMAIN_SYS,
>> +    SPEL_DOMAIN_SOC,
>> +    SPEL_DOMAIN_CL0,
>> +    SPEL_DOMAIN_CL1,
>> +    SPEL_DOMAIN_CL2,
>> +    SPEL_DOMAIN_IGPU,
>> +    SPEL_DOMAIN_DGPU,
>> +    SPEL_DOMAIN_NSP,
>> +    SPEL_DOMAIN_MMCX,
>> +    SPEL_DOMAIN_INFRA,
>> +    SPEL_DOMAIN_DRAM,
>> +    SPEL_DOMAIN_MDM,
>> +    SPEL_DOMAIN_WLAN,
>> +    SPEL_DOMAIN_USB1,
>> +    SPEL_DOMAIN_USB2,
>> +    SPEL_DOMAIN_USB3,
>> +    SPEL_DOMAIN_MAX,
>> +};
>> +
>> +/* Power limit IDs */
>> +enum spel_power_limit_id {
>> +    POWER_LIMIT1,
>> +    POWER_LIMIT2,
>> +    POWER_LIMIT3,
>> +    POWER_LIMIT4,
>> +    NR_POWER_LIMITS,
>> +};
>> +
>> +/* Unit types for conversion */
>> +enum unit_type {
>> +    POWER_UNIT,
>> +    ENERGY_UNIT,
>> +    TIME_UNIT,
>> +};
>> +
>> +/* Power limit operation types */
>> +enum pl_ops_type {
>> +    PL_LIMIT,
>> +    PL_TIME_WINDOW,
>> +};
>> +
>> +static const char *pl_names[NR_POWER_LIMITS] = {
>> +    [POWER_LIMIT1] = "pl1",
>> +    [POWER_LIMIT2] = "pl2",
>> +    [POWER_LIMIT3] = "pl3",
>> +    [POWER_LIMIT4] = "pl4",
>> +};
> 
> Do you want to use NR_POWER_LIMITS instead of ARRAY_SIZE() ?

ACK, I will move to ARRAY_SIZE>
> Please unify the naming NR_POWER_LIMITS -> POWER_LIMITS_MAX

ACK

> 
>> +static const char *const spel_domain_names[] = {
>> +    "sys", "soc", "cl0", "cl1", "cl2", "igpu", "dgpu", "nsp",
>> +    "mmcx", "infra", "dram", "mdm", "wlan", "usb1", "usb2", "usb3",
>> +};
>> +
>> +/* Domain register offsets in node base */
>> +static const u32 domain_offsets[SPEL_DOMAIN_MAX] = {
>> +    [SPEL_DOMAIN_SYS]    = 0x40,
>> +    [SPEL_DOMAIN_SOC]    = 0x00,
>> +    [SPEL_DOMAIN_CL0]    = 0x5C,
>> +    [SPEL_DOMAIN_CL1]    = 0x60,
>> +    [SPEL_DOMAIN_CL2]    = 0x64,
>> +    [SPEL_DOMAIN_IGPU]    = 0x08,
>> +    [SPEL_DOMAIN_DGPU]    = 0x44,
>> +    [SPEL_DOMAIN_NSP]    = 0x0C,
>> +    [SPEL_DOMAIN_MMCX]    = 0x10,
>> +    [SPEL_DOMAIN_INFRA]    = 0x18,
>> +    [SPEL_DOMAIN_DRAM]    = 0x1C,
>> +    [SPEL_DOMAIN_MDM]    = 0x48,
>> +    [SPEL_DOMAIN_WLAN]    = 0x4C,
>> +    [SPEL_DOMAIN_USB1]    = 0x50,
>> +    [SPEL_DOMAIN_USB2]    = 0x54,
>> +    [SPEL_DOMAIN_USB3]    = 0x58,
>> +};
> 
> Same comment

ACK

> 
>> +/**
>> + * struct spel_constraint_info - Power limit constraint information
>> + * @limit_offset:    Register offset for power limit value
>> + * @time_window_offset:    Register offset for time window
>> + * @supported_mask:    Bit mask in capability register
> 
> Where is 'supported_mask' initialized?

It is part of constraint_info table and used as bit mask to compare 
againt capability config register value during init for each PL for 
different domains.

> 
>> + * @domain_id:        Domain this constraint applies to
>> + * @pl_id:        Power limit ID (PL1, PL2, etc.)
> 
> [ ... ]
> 
>> +
>> +/**
>> + * struct spel_domain - SPEL power domain
>> + * @power_zone:        Powercap zone
>> + * @lock:        Mutex protecting register access
>> + * @sp:            Parent system
>> + * @status_reg:        Energy counter register
>> + * @pl_name:        Power limit names
>> + * @name:        Domain name
>> + * @id:            Domain type ID
>> + */
>> +struct spel_domain {
> 
> [ ... ]
> 
>> +    struct spel_system *sp;
> 
> [ ... ]
> 
>> +struct spel_system {
>> +    struct spel_domain *domains;
> 
> [ ... ]
> 
>> +};
> 
> There is a cyclic dependency between struct spel_system <-> struct 
> spel_domain. Could it be solved ?

ACK

> 
>> +#define power_zone_to_spel_domain(_zone) \
>> +    container_of(_zone, struct spel_domain, power_zone)
>> +
>> +/* Helper functions */
>> +static bool is_pl_valid(struct spel_domain *sd, int pl)
>> +{
>> +    if (pl < POWER_LIMIT1 || pl >= NR_POWER_LIMITS)
>> +        return false;
> 
> The call to this function is strange. It is like you don't trust your 
> own code.

ACK, I will remove it in v2

> 
>> +    return sd->pl_name[pl] ? true : false;
>> +}
>> +
> 
> [ ... ]
> 
>> +
>> +    switch (pl_op) {
>> +    case PL_LIMIT:
>> +        value &= POWER_LIMIT_MASK;
>> +        if (xlate)
>> +            *data = spel_unit_xlate(sd, POWER_UNIT, value, 0);
>> +        else
>> +            *data = value;
>> +        break;
>> +    case PL_TIME_WINDOW:
>> +        /* Decode time window: bits [22:16] are upper 7 bits, [14:0] 
>> are lower 15 bits */
>> +        value = ((value & TIME_WINDOW_MASK_H) >> 16 << 15) |
>> +            (value & TIME_WINDOW_MASK_L);
> 
> [ ... ]
> 
>> +        reg_val = (reg_val & ~POWER_LIMIT_MASK) | new_val;
>> +
> 
> [ ... ]
> 
>> +        if (new_val == 0)
>> +            reg_val &= ~POWER_LIMIT_ENABLE;
>> +        else
>> +            reg_val |= POWER_LIMIT_ENABLE;
>> +        break;
> 
> [ ... ]
> 
>> +    case PL_TIME_WINDOW:
>> +        /*
>> +         * Encode time window: upper 7 bits to [22:16], lower 15 bits 
>> to [14:0]
>> +         * Time window register is separate from limit register 
>> (different offset),
>> +         * so we write only the time window bits without preserving 
>> any enable bit.
>> +         */
>> +        new_val = spel_unit_xlate(sd, TIME_UNIT, value, 1);
>> +        reg_val = (((new_val >> 15) & 0x7F) << 16) |
>> +              (new_val & 0x7FFF);
> 
> 
> The trend today is to use the FIELD_* macros for bits ops

ACK

> 
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>> +    writel(reg_val, reg_addr);
>> +    return 0;
>> +}
>> +
>> +/* Powercap zone operations */
>> +static int spel_get_energy_counter(struct powercap_zone *power_zone, 
>> u64 *energy_raw)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
>> +    u64 value;
>> +
>> +    value = readl(sd->status_reg);
> 
> nit: seems an line in between would help for the readability

ACK

> 
>> +    *energy_raw = spel_unit_xlate(sd, ENERGY_UNIT, value, 0);
>> +
>> +    return 0;
>> +}
>> +
>> +static int spel_get_max_energy_counter(struct powercap_zone *pcd_dev, 
>> u64 *energy)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(pcd_dev);
>> +
>> +    *energy = spel_unit_xlate(sd, ENERGY_UNIT, ENERGY_STATUS_MASK, 0);
> 
> nit: missing line

ACK>
>> +    return 0;
>> +}
>> +
>> +static int spel_release_zone(struct powercap_zone *power_zone)
>> +{
>> +    return 0;
>> +}
>> +
>> +static int spel_find_nr_power_limit(struct spel_domain *sd)
>> +{
>> +    int i, nr_pl = 0;
>> +
>> +    for (i = 0; i < NR_POWER_LIMITS; i++) {
>> +        if (is_pl_valid(sd, i))
>> +            nr_pl++;
>> +    }
>> +
>> +    return nr_pl;
>> +}
>> +
>> +static const struct powercap_zone_ops zone_ops = {
>> +    .get_energy_uj = spel_get_energy_counter,
>> +    .get_max_energy_range_uj = spel_get_max_energy_counter,
>> +    .release = spel_release_zone,
>> +};
>> +
>> +/* Constraint operations */
>> +static int spel_constraint_to_pl(struct spel_domain *sd, int cid)
>> +{
>> +    int i, j;
> 
> 'j' name is misleading because it is usually used for nested 'for' blocks

ACK

> 
>> +    for (i = POWER_LIMIT1, j = 0; i < NR_POWER_LIMITS; i++) {
> 
> Do not rely on POWER_LIMIT1 because if someday it is moved in the enum, 
> all the code assuming it is zero will be broken

ACK

> 
>> +        if (is_pl_valid(sd, i) && j++ == cid)
>  > +            return i;> +    }
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +static int spel_set_power_limit(struct powercap_zone *power_zone, int 
>> cid,
>> +                u64 power_limit)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
>> +    int id;
>> +
>> +    id = spel_constraint_to_pl(sd, cid);
>> +    if (id < 0)
>> +        return id;
>> +
>> +    return spel_write_pl_data(sd, id, PL_LIMIT, power_limit);
>> +}
>> +
>> +static int spel_get_power_limit(struct powercap_zone *power_zone, int 
>> cid,
>> +                u64 *data)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
>> +    u64 val;
>> +    int ret, id;
>> +
>> +    id = spel_constraint_to_pl(sd, cid);
>> +    if (id < 0)
>> +        return id;
>> +
>> +    ret = spel_read_pl_data(sd, id, PL_LIMIT, true, &val);
>> +    if (!ret)
>> +        *data = val;
>> +
>> +    return ret;
>> +}
>> +
>> +static int spel_set_time_window(struct powercap_zone *power_zone, int 
>> cid,
>> +                u64 window)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
>> +    int id;
>> +
>> +    id = spel_constraint_to_pl(sd, cid);
>> +    if (id < 0)
>> +        return id;
>> +
>> +    return spel_write_pl_data(sd, id, PL_TIME_WINDOW, window);
>> +}
>> +
>> +static int spel_get_time_window(struct powercap_zone *power_zone, int 
>> cid,
>> +                u64 *data)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
>> +    u64 val;
>> +    int ret, id;
>> +
>> +    id = spel_constraint_to_pl(sd, cid);
>> +    if (id < 0)
>> +        return id;
>> +
>> +    ret = spel_read_pl_data(sd, id, PL_TIME_WINDOW, true, &val);
>> +    if (!ret)
>> +        *data = val;
>> +
>> +    return ret;
>> +}
>> +
>> +static const char *spel_get_constraint_name(struct powercap_zone 
>> *power_zone,
>> +                        int cid)
>> +{
>> +    struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
>> +    int id;
>> +
>> +    id = spel_constraint_to_pl(sd, cid);
>> +    if (id >= 0)
>> +        return sd->pl_name[id];
>> +
>> +    return NULL;
>> +}
>> +
>> +static const struct powercap_zone_constraint_ops constraint_ops = {
>> +    .set_power_limit_uw = spel_set_power_limit,
>> +    .get_power_limit_uw = spel_get_power_limit,
>> +    .set_time_window_us = spel_set_time_window,
>> +    .get_time_window_us = spel_get_time_window,
>> +    .get_name = spel_get_constraint_name,
>> +};
>> +
>> +static void spel_init_domains(struct spel_system *sp)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < SPEL_DOMAIN_MAX; i++) {
>> +        struct spel_domain *sd = &sp->domains[i];
>> +
>> +        sd->sp = sp;
>> +        snprintf(sd->name, SPEL_DOMAIN_NAME_LENGTH, "%s",
>> +             spel_domain_names[i]);
>> +        sd->id = i;
>> +        sd->status_reg = sp->node_base + domain_offsets[i];
>> +
>> +        /* PL1 is always supported (required for powercap 
>> registration) */
>> +        sp->limits[i] = BIT(POWER_LIMIT1);
>> +        sd->pl_name[POWER_LIMIT1] = pl_names[POWER_LIMIT1];
>> +    }
>> +}
>> +
>> +static int spel_check_unit(struct spel_system *sp)
>> +{
>> +    u32 value, shift;
>> +
>> +    /* Read power_unit and time_unit from offset 0x0 */
>> +    value = readl(sp->config_base);
>> +
>> +    /*
>> +     * Unit calculation: 1 / (2^shift)
>> +     * Masks limit: TIME_UNIT (4 bits, max 15), POWER_UNIT (3 bits, 
>> max 7).
>> +     */
>> +    shift = (value & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
>> +    sp->power_unit = 1000000 / (1 << shift);
>> +
>> +    shift = (value & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
>> +    sp->time_unit = 1000000 / (1 << shift);
>> +
>> +    /* Read energy_unit from ENERGY_RPT_UNIT_OFFSET */
>> +    value = readl(sp->config_base + ENERGY_RPT_UNIT_OFFSET);
>> +
>> +    /*
>> +     * Unit calculation: 1 / (2^shift)
>> +     * Masks limit: ENERGY_UNIT (4 bits, max 15).
>> +     */
>> +    shift = (value & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
>> +    sp->energy_unit = ENERGY_UNIT_SCALE * 1000000 / (1 << shift);
>> +
>> +    dev_dbg(sp->dev, "Units: energy=%dnJ, time=%dus, power=%duW\n",
>> +        sp->energy_unit, sp->time_unit, sp->power_unit);
>> +
>> +    return 0;
>> +}
>> +
>> +static void spel_detect_powerlimit(struct spel_domain *sd)
>> +{
>> +    struct spel_system *sp = sd->sp;
>> +    u32 capabilities;
>> +    int i, j;
>> +
>> +    capabilities = readl(sp->config_base + LIMITS_CAPABILITY_OFFSET);
>> +
>> +    /* Detect power limits from hardware capabilities */
>> +    for (i = POWER_LIMIT2; i < NR_POWER_LIMITS; i++) {
> 
> For my understanding, why is it starting at POWER_LIMIT2 ?

Since It is failing to register a powercap zone with 0 constraint, I 
need to add alteast one constraint for each domain. PL1 is added by 
default for all domains.

> 
>> +        for (j = 0; j < ARRAY_SIZE(constraints); j++) {
>> +            struct spel_constraint_info *ci = &constraints[j];
>> +
>> +            if (ci->domain_id == sd->id && ci->pl_id == i) {
>> +                if (capabilities & ci->supported_mask) {
>> +                    sp->limits[sd->id] |= BIT(i);
>> +                    sd->pl_name[i] = pl_names[i];
> 
> So, that explains the is_pl_valid()
> 
> Please do not use duplicated array with non-NULL pointer checks
> 
> Or sd->pl_name[] contains all the *valids* power limits, so its size is 
> different than (or equal to) pl_names. Or it is a fixed array with a 
> structure containing a flag telling if it is enabled or not.
> 
> No need to duplicate the array

ACK

> 
>> +                }
>> +                break;
>> +            }
>> +        }
>> +    }
>> +}
>> +
> 
> [ ... ]
> 
>> +static void spel_remove(struct platform_device *pdev)
>> +{
>> +    struct spel_system *sp = platform_get_drvdata(pdev);
>> +    int i;
>> +
>> +    if (!sp)
>> +        return;
> 
> Why test if the value is correct? There is no reason it changed after 
> 'probe' was successful

ACK

Thanks,
Manaf

> 
>> +
>> +    /* Unregister in reverse order: children first, then SOC, then 
>> SYS */
>> +    for (i = SPEL_DOMAIN_MAX - 1; i >= 0; i--)
>> +        powercap_unregister_zone(sp->control_type, &sp- 
>> >domains[i].power_zone);
>> +
>> +    powercap_unregister_control_type(sp->control_type);
>> +}
>> +
>> +static const struct of_device_id spel_of_match[] = {
>> +    { .compatible = "qcom,spel" },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, spel_of_match);
>> +
>> +static struct platform_driver spel_driver = {
>> +    .probe = spel_probe,
>> +    .remove = spel_remove,
>> +    .driver = {
>> +        .name = "qcom_spel",
>> +        .of_match_table = spel_of_match,
>> +    },
>> +};
>> +
>> +module_platform_driver(spel_driver);
>> +
>> +MODULE_DESCRIPTION("Qualcomm SPEL Powercap Driver");
>> +MODULE_LICENSE("GPL");
>>
> 

[PATCH 3/3] arm64: dts: qcom: glymur: Enable SPEL powercap driver

[Manaf Meethalavalappu Pallikunhi]

The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
based power monitoring and limiting capabilities for various power
domains including System, SoC, CPU clusters, GPU, and various other
subsystems for glymur.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/glymur.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
index 72c7dc6e4f09..c0ba8740ae41 100644
--- a/arch/arm64/boot/dts/qcom/glymur.dtsi
+++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
@@ -5033,6 +5033,16 @@ spmi_bus2: spmi@c48000 {
 			};
 		};
 
+		power-controller@ef3b000 {
+			compatible = "qcom,spel";
+			reg = <0x0 0x0ef3e000 0x0 0x1000>,
+			      <0x0 0x0ef3d000 0x0 0x1000>,
+			      <0x0 0x0ef3b000 0x0 0x1000>;
+			reg-names = "nodes",
+				    "constraints",
+				    "config";
+		};
+
 		tlmm: pinctrl@f100000 {
 			compatible = "qcom,glymur-tlmm";
 			reg = <0x0 0x0f100000 0x0 0xf00000>;

-- 
2.43.0

Re: [PATCH 3/3] arm64: dts: qcom: glymur: Enable SPEL powercap driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The unit-address of the `power-controller` node (`@ef3b000`) does not match the first address specified in the `reg` property (`0x0ef3e000`).
--

commit 2d003127cde30ba8cb78978e9f333e066d842620
Author: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
arm64: dts: qcom: glymur: Enable SPEL powercap driver

This commit adds the Qualcomm SoC Power and Electrical Limits (SPEL) node
to the glymur device tree. This enables hardware-based power monitoring and
limiting capabilities for various power domains.

> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index 72c7dc6e4f098..c0ba8740ae413 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
[ ... ]
> +		power-controller@ef3b000 {
> +			compatible = "qcom,spel";
> +			reg = <0x0 0x0ef3e000 0x0 0x1000>,

This isn't a bug, but the unit-address in the node name (@ef3b000) does not
match the first address specified in the reg property (0x0ef3e000). 

Should the node name be updated to power-controller@ef3e000 to match the
first reg entry and avoid compiler warnings?

> +			      <0x0 0x0ef3d000 0x0 0x1000>,
> +			      <0x0 0x0ef3b000 0x0 0x1000>;
> +			reg-names = "nodes",
> +				    "constraints",
> +				    "config";
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-qcom_spel_driver_upstream-v1-0-75356d1b7f94@oss.qualcomm.com?part=3

Re: [PATCH 3/3] arm64: dts: qcom: glymur: Enable SPEL powercap driver

[Krzysztof Kozlowski]

On 19/05/2026 12:49, Manaf Meethalavalappu Pallikunhi wrote:
> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
> based power monitoring and limiting capabilities for various power
> domains including System, SoC, CPU clusters, GPU, and various other
> subsystems for glymur.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/glymur.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index 72c7dc6e4f09..c0ba8740ae41 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
> @@ -5033,6 +5033,16 @@ spmi_bus2: spmi@c48000 {
>  			};
>  		};
>  
> +		power-controller@ef3b000 {

Never properly build tested. Please read carefully either kernel docs or
internal guide which tells you what steps you are supposed to do.

NAK

Best regards,
Krzysztof