1. Patchew
  2. linux
  3. [PATCH v4 0/5] Add driver for EC found on Qualcomm reference devices
  4. View patch

[PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices

Anvesh Jain P posted 5 patches 3 weeks, 4 days ago
There is a newer version of this series
[PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Anvesh Jain P 3 weeks, 4 days ago 
From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>

Add Embedded controller driver support for Hamoa/Purwa/Glymur qualcomm
reference boards. It handles fan control, temperature sensors, access
to EC state changes and supports reporting suspend entry/exit to the
EC.

Co-developed-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
Co-developed-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
---
 MAINTAINERS                            |   8 +
 drivers/platform/arm64/Kconfig         |  12 +
 drivers/platform/arm64/Makefile        |   1 +
 drivers/platform/arm64/qcom-hamoa-ec.c | 468 +++++++++++++++++++++++++++++++++
 4 files changed, 489 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2882a67bdf6d..9657c384be44 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21932,6 +21932,14 @@ S:	Supported
 W:	https://wireless.wiki.kernel.org/en/users/Drivers/wcn36xx
 F:	drivers/net/wireless/ath/wcn36xx/
 
+QUALCOMM HAMOA EMBEDDED CONTROLLER DRIVER
+M:	Sibi Sankar <sibi.sankar@oss.qualcomm.com>
+M:	Anvesh Jain P <anvesh.p@oss.qualcomm.com>
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
+F:	drivers/platform/arm64/qcom-hamoa-ec.c
+
 QUANTENNA QTNFMAC WIRELESS DRIVER
 M:	Igor Mitsyanko <imitsyanko@quantenna.com>
 R:	Sergey Matyukevich <geomatsi@gmail.com>
diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
index 10f905d7d6bf..025cdf091f9e 100644
--- a/drivers/platform/arm64/Kconfig
+++ b/drivers/platform/arm64/Kconfig
@@ -90,4 +90,16 @@ config EC_LENOVO_THINKPAD_T14S
 
 	  Say M or Y here to include this support.
 
+config EC_QCOM_HAMOA
+	tristate "Embedded Controller driver for Qualcomm Hamoa/Glymur reference devices"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on I2C
+	help
+	  Say M or Y here to enable the Embedded Controller driver for Qualcomm
+	  Snapdragon-based Hamoa/Glymur reference devices. The driver handles fan
+	  control, temperature sensors, access to EC state changes and supports
+	  reporting suspend entry/exit to the EC.
+
+	  This driver currently supports Hamoa/Purwa/Glymur reference devices.
+
 endif # ARM64_PLATFORM_DEVICES
diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
index 60c131cff6a1..7681be4a46e9 100644
--- a/drivers/platform/arm64/Makefile
+++ b/drivers/platform/arm64/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_EC_ACER_ASPIRE1)	+= acer-aspire1-ec.o
 obj-$(CONFIG_EC_HUAWEI_GAOKUN)	+= huawei-gaokun-ec.o
 obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
 obj-$(CONFIG_EC_LENOVO_THINKPAD_T14S) += lenovo-thinkpad-t14s.o
+obj-$(CONFIG_EC_QCOM_HAMOA) += qcom-hamoa-ec.o
diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
new file mode 100644
index 000000000000..452775e765e8
--- /dev/null
+++ b/drivers/platform/arm64/qcom-hamoa-ec.c
@@ -0,0 +1,468 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Maya Matuszczyk <maccraft123mc@gmail.com>
+ * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/thermal.h>
+
+#define EC_SCI_EVT_READ_CMD	0x05
+#define EC_FW_VERSION_CMD	0x0e
+#define EC_MODERN_STANDBY_CMD	0x23
+#define EC_FAN_DBG_CONTROL_CMD	0x30
+#define EC_SCI_EVT_CONTROL_CMD	0x35
+#define EC_THERMAL_CAP_CMD	0x42
+
+#define EC_FW_VERSION_RESP_LEN	4
+#define EC_THERMAL_CAP_RESP_LEN	3
+#define EC_FAN_DEBUG_CMD_LEN	6
+#define EC_FAN_SPEED_DATA_SIZE	4
+
+#define EC_MODERN_STANDBY_ENTER	0x01
+#define EC_MODERN_STANDBY_EXIT	0x00
+
+#define EC_FAN_DEBUG_MODE_OFF   0
+#define EC_FAN_DEBUG_MODE_ON    BIT(0)
+#define EC_FAN_ON               BIT(1)
+#define EC_FAN_DEBUG_TYPE_PWM   BIT(2)
+#define EC_MAX_FAN_CNT		2
+#define EC_FAN_NAME_SIZE	20
+#define EC_FAN_MAX_PWM		255
+
+enum qcom_ec_sci_events {
+	EC_FAN1_STATUS_CHANGE_EVT = 0x30,
+	EC_FAN2_STATUS_CHANGE_EVT,
+	EC_FAN1_SPEED_CHANGE_EVT,
+	EC_FAN2_SPEED_CHANGE_EVT,
+	EC_NEW_LUT_SET_EVT,
+	EC_FAN_PROFILE_SWITCH_EVT,
+	EC_THERMISTOR_1_THRESHOLD_CROSS_EVT,
+	EC_THERMISTOR_2_THRESHOLD_CROSS_EVT,
+	EC_THERMISTOR_3_THRESHOLD_CROSS_EVT,
+	/* Reserved: 0x39 - 0x3c/0x3f */
+	EC_RECOVERED_FROM_RESET_EVT = 0x3d,
+};
+
+struct qcom_ec_version {
+	u8 main_version;
+	u8 sub_version;
+	u8 test_version;
+};
+
+struct qcom_ec_thermal_cap {
+#define EC_THERMAL_FAN_CNT(x)		(FIELD_GET(GENMASK(1, 0), (x)))
+#define EC_THERMAL_FAN_TYPE(x)		(FIELD_GET(GENMASK(4, 2), (x)))
+#define EC_THERMAL_THERMISTOR_MASK(x)	(FIELD_GET(GENMASK(7, 0), (x)))
+	u8 fan_cnt;
+	u8 fan_type;
+	u8 thermistor_mask;
+};
+
+struct qcom_ec_cooling_dev {
+	struct thermal_cooling_device *cdev;
+	struct device *parent_dev;
+	u8 fan_id;
+	u8 state;
+};
+
+struct qcom_ec {
+	struct qcom_ec_cooling_dev *ec_cdev;
+	struct qcom_ec_thermal_cap thermal_cap;
+	struct qcom_ec_version version;
+	struct i2c_client *client;
+};
+
+static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
+{
+	int ret;
+
+	ret = i2c_smbus_read_i2c_block_data(ec->client, cmd, resp_len, resp);
+
+	if (ret < 0)
+		return ret;
+	else if (ret == 0 || ret == 0xff)
+		return -EOPNOTSUPP;
+
+	if (resp[0] >= resp_len)
+		return -EINVAL;
+
+	return 0;
+}
+
+/*
+ * EC Device Firmware Version:
+ *
+ * Read Response:
+ * ----------------------------------------------------------------------
+ * | Offset	| Name		| Description				|
+ * ----------------------------------------------------------------------
+ * | 0x00	| Byte count	| Number of bytes in response		|
+ * |		|		| (exluding byte count)			|
+ * ----------------------------------------------------------------------
+ * | 0x01	| Test-version	| Test-version of EC firmware		|
+ * ----------------------------------------------------------------------
+ * | 0x02	| Sub-version	| Sub-version of EC firmware		|
+ * ----------------------------------------------------------------------
+ * | 0x03	| Main-version	| Main-version of EC firmware		|
+ * ----------------------------------------------------------------------
+ *
+ */
+static int qcom_ec_read_fw_version(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct qcom_ec *ec = i2c_get_clientdata(client);
+	struct qcom_ec_version *version = &ec->version;
+	u8 resp[EC_FW_VERSION_RESP_LEN];
+	int ret;
+
+	ret = qcom_ec_read(ec, EC_FW_VERSION_CMD, EC_FW_VERSION_RESP_LEN, resp);
+	if (ret < 0)
+		return ret;
+
+	version->main_version = resp[3];
+	version->sub_version = resp[2];
+	version->test_version = resp[1];
+
+	dev_dbg(dev, "EC Version %d.%d.%d\n",
+		version->main_version, version->sub_version, version->test_version);
+
+	return 0;
+}
+
+/*
+ * EC Device Thermal Capabilities:
+ *
+ * Read Response:
+ * ------------------------------------------------------------------------------
+ * | Offset		| Name		| Description				|
+ * ------------------------------------------------------------------------------
+ * | 0x00		| Byte count	| Number of bytes in response		|
+ * |			|		| (exluding byte count)			|
+ * ------------------------------------------------------------------------------
+ * | 0x02 (LSB)	| EC Thermal	| Bit 0-1: Number of fans		|
+ * | 0x3		| Capabilities	| Bit 2-4: Type of fan			|
+ * |			|		| Bit 5-6: Reserved			|
+ * |			|		| Bit 7: Data Valid/Invalid		|
+ * |			|		|	 (Valid - 1, Invalid - 0)	|
+ * |			|		| Bit 8-15: Thermistor 0 - 7 presence	|
+ * |			|		|	    (0 present, 1 absent)	|
+ * ------------------------------------------------------------------------------
+ *
+ */
+static int qcom_ec_thermal_capabilities(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct qcom_ec *ec = i2c_get_clientdata(client);
+	struct qcom_ec_thermal_cap *cap = &ec->thermal_cap;
+	u8 resp[EC_THERMAL_CAP_RESP_LEN];
+	int ret;
+
+	ret = qcom_ec_read(ec, EC_THERMAL_CAP_CMD, EC_THERMAL_CAP_RESP_LEN, resp);
+	if (ret < 0)
+		return ret;
+
+	cap->fan_cnt = min(EC_MAX_FAN_CNT, EC_THERMAL_FAN_CNT(resp[1]));
+	cap->fan_type = EC_THERMAL_FAN_TYPE(resp[1]);
+	cap->thermistor_mask = EC_THERMAL_THERMISTOR_MASK(resp[2]);
+
+	dev_dbg(dev, "Fan count: %d Fan Type: %d Thermistor Mask: %d\n",
+		cap->fan_cnt, cap->fan_type, cap->thermistor_mask);
+
+	return 0;
+}
+
+static irqreturn_t qcom_ec_irq(int irq, void *data)
+{
+	struct qcom_ec *ec = data;
+	struct device *dev = &ec->client->dev;
+	int val;
+
+	val = i2c_smbus_read_byte_data(ec->client, EC_SCI_EVT_READ_CMD);
+	if (val < 0) {
+		dev_err(dev, "Failed to read EC SCI Event: %d\n", val);
+		return IRQ_HANDLED;
+	}
+
+	switch (val) {
+	case EC_FAN1_STATUS_CHANGE_EVT:
+		dev_dbg(dev, "Fan1 status changed\n");
+		break;
+	case EC_FAN2_STATUS_CHANGE_EVT:
+		dev_dbg(dev, "Fan2 status changed\n");
+		break;
+	case EC_FAN1_SPEED_CHANGE_EVT:
+		dev_dbg(dev, "Fan1 speed crossed low/high trip point\n");
+		break;
+	case EC_FAN2_SPEED_CHANGE_EVT:
+		dev_dbg(dev, "Fan2 speed crossed low/high trip point\n");
+		break;
+	case EC_NEW_LUT_SET_EVT:
+		dev_dbg(dev, "New LUT set\n");
+		break;
+	case EC_FAN_PROFILE_SWITCH_EVT:
+		dev_dbg(dev, "FAN Profile switched\n");
+		break;
+	case EC_THERMISTOR_1_THRESHOLD_CROSS_EVT:
+		dev_dbg(dev, "Thermistor 1 threshold crossed\n");
+		break;
+	case EC_THERMISTOR_2_THRESHOLD_CROSS_EVT:
+		dev_dbg(dev, "Thermistor 2 threshold crossed\n");
+		break;
+	case EC_THERMISTOR_3_THRESHOLD_CROSS_EVT:
+		dev_dbg(dev, "Thermistor 3 threshold crossed\n");
+		break;
+	case EC_RECOVERED_FROM_RESET_EVT:
+		dev_dbg(dev, "EC recovered from reset\n");
+		break;
+	default:
+		dev_dbg(dev, "Unknown EC event: %d\n", val);
+		break;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int qcom_ec_sci_evt_control(struct device *dev, bool enable)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 control = enable ? 1 : 0;
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, EC_SCI_EVT_CONTROL_CMD, control);
+
+	return ret;
+}
+
+static int qcom_ec_fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	*state = EC_FAN_MAX_PWM;
+
+	return 0;
+}
+
+static int qcom_ec_fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	struct qcom_ec_cooling_dev *ec_cdev = cdev->devdata;
+
+	*state = ec_cdev->state;
+
+	return 0;
+}
+
+/*
+ * Fan Debug control command:
+ *
+ * Command Payload:
+ * --------------------------------------------------------------------------------------
+ * | Offset		| Name		| Description					|
+ * --------------------------------------------------------------------------------------
+ * | 0x00		| Command	| Fan control command				|
+ * --------------------------------------------------------------------------------------
+ * | 0x01		| Fan ID	| 0x1 : Fan 1					|
+ * |			|		| 0x2 : Fan 2					|
+ * --------------------------------------------------------------------------------------
+ * | 0x02		| Byte count = 4| Size of data to set fan speed			|
+ * --------------------------------------------------------------------------------------
+ * | 0x03		| Mode		| Bit 0: Debug Mode On/Off (0 - OFF, 1 - ON )	|
+ * |			|		| Bit 1: Fan On/Off (0 - Off, 1 - ON)		|
+ * |			|		| Bit 2: Debug Type (0 - RPM, 1 - PWM)		|
+ * --------------------------------------------------------------------------------------
+ * | 0x04 (LSB)	| Speed in RPM	| RPM value, if mode selected is RPM		|
+ * | 0x05		|		|						|
+ * --------------------------------------------------------------------------------------
+ * | 0x06		| Speed in PWM	| PWM value, if mode selected is PWM (0 - 255)	|
+ * ______________________________________________________________________________________
+ *
+ */
+static int qcom_ec_fan_debug_mode_off(struct qcom_ec_cooling_dev *ec_cdev)
+{
+	struct device *dev = ec_cdev->parent_dev;
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 request[6] = { ec_cdev->fan_id, EC_FAN_SPEED_DATA_SIZE,
+			  EC_FAN_DEBUG_MODE_OFF, 0, 0, 0 };
+	int ret;
+
+	ret = i2c_smbus_write_i2c_block_data(client, EC_FAN_DBG_CONTROL_CMD,
+					     sizeof(request), request);
+	if (ret)
+		dev_err(dev, "Failed to turn off fan%d debug mode: %d\n",
+			ec_cdev->fan_id, ret);
+
+	return ret;
+}
+
+static int qcom_ec_fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
+{
+	struct qcom_ec_cooling_dev *ec_cdev = cdev->devdata;
+	struct device *dev = ec_cdev->parent_dev;
+	struct i2c_client *client = to_i2c_client(dev);
+
+	u8 request[6] = { ec_cdev->fan_id, EC_FAN_SPEED_DATA_SIZE,
+			  EC_FAN_DEBUG_MODE_ON | EC_FAN_ON | EC_FAN_DEBUG_TYPE_PWM,
+			  0, 0, state };
+	int ret;
+
+	ret = i2c_smbus_write_i2c_block_data(client, EC_FAN_DBG_CONTROL_CMD,
+					     sizeof(request), request);
+	if (ret) {
+		dev_err(dev, "Failed to set fan pwm: %d\n", ret);
+		return ret;
+	}
+
+	ec_cdev->state = state;
+
+	return 0;
+}
+
+static const struct thermal_cooling_device_ops qcom_ec_thermal_ops = {
+	.get_max_state = qcom_ec_fan_get_max_state,
+	.get_cur_state = qcom_ec_fan_get_cur_state,
+	.set_cur_state = qcom_ec_fan_set_cur_state,
+};
+
+static int qcom_ec_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, EC_MODERN_STANDBY_CMD, EC_MODERN_STANDBY_ENTER);
+
+	return ret;
+}
+
+static int qcom_ec_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, EC_MODERN_STANDBY_CMD, EC_MODERN_STANDBY_EXIT);
+
+	return ret;
+}
+
+static int qcom_ec_probe(struct i2c_client *client)
+{
+	struct qcom_ec_cooling_dev *cdev;
+	struct device *dev = &client->dev;
+	struct qcom_ec *ec;
+	int ret, i;
+
+	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+	if (!ec)
+		return -ENOMEM;
+
+	ec->client = client;
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL, qcom_ec_irq,
+					IRQF_ONESHOT, "qcom_ec", ec);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get irq\n");
+
+	i2c_set_clientdata(client, ec);
+
+	ret = qcom_ec_read_fw_version(dev);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");
+
+	ret = qcom_ec_thermal_capabilities(dev);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to read thermal capabilities\n");
+
+	ret = qcom_ec_sci_evt_control(dev, true);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to enable SCI events\n");
+
+	ec->ec_cdev = devm_kcalloc(dev, ec->thermal_cap.fan_cnt, sizeof(*ec->ec_cdev), GFP_KERNEL);
+	if (!ec->ec_cdev)
+		return -ENOMEM;
+
+	for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
+		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
+		char name[EC_FAN_NAME_SIZE];
+
+		snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
+		ec_cdev->fan_id = i + 1;
+		ec_cdev->parent_dev = dev;
+
+		ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
+								&qcom_ec_thermal_ops);
+		if (IS_ERR(ec_cdev->cdev)) {
+			dev_err_probe(dev, PTR_ERR(cdev),
+				      "Thermal cooling device registration failed\n");
+			ret = -EINVAL;
+			goto unroll_cooling_dev;
+		}
+	}
+
+	return 0;
+
+unroll_cooling_dev:
+	for (i--; i >= 0; i--) {
+		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
+
+		if (ec_cdev->cdev) {
+			thermal_cooling_device_unregister(ec_cdev->cdev);
+			ec_cdev->cdev = NULL;
+		}
+	}
+
+	return ret;
+}
+
+static void qcom_ec_remove(struct i2c_client *client)
+{
+	struct qcom_ec *ec = i2c_get_clientdata(client);
+	struct device *dev = &client->dev;
+	int ret;
+
+	ret = qcom_ec_sci_evt_control(dev, false);
+	if (ret < 0)
+		dev_err(dev, "Failed to disable SCI events: %d\n", ret);
+
+	for (int i = 0; i < ec->thermal_cap.fan_cnt; i++) {
+		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
+
+		qcom_ec_fan_debug_mode_off(ec_cdev);
+
+		if (ec_cdev->cdev) {
+			thermal_cooling_device_unregister(ec_cdev->cdev);
+			ec_cdev->cdev = NULL;
+		}
+	}
+}
+
+static const struct of_device_id qcom_ec_of_match[] = {
+	{ .compatible = "qcom,hamoa-crd-ec" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_ec_of_match);
+
+static const struct i2c_device_id qcom_ec_i2c_id_table[] = {
+	{ "qcom-hamoa-ec", },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, qcom_ec_i2c_id_table);
+
+static DEFINE_SIMPLE_DEV_PM_OPS(qcom_ec_pm_ops,
+		qcom_ec_suspend,
+		qcom_ec_resume);
+
+static struct i2c_driver qcom_ec_i2c_driver = {
+	.driver = {
+		.name = "qcom-hamoa-ec",
+		.of_match_table = qcom_ec_of_match,
+		.pm = &qcom_ec_pm_ops
+	},
+	.probe = qcom_ec_probe,
+	.remove = qcom_ec_remove,
+	.id_table = qcom_ec_i2c_id_table,
+};
+module_i2c_driver(qcom_ec_i2c_driver);
+
+MODULE_DESCRIPTION("QCOM Hamoa Embedded Controller");
+MODULE_LICENSE("GPL");

-- 
2.34.1
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by kernel test robot 3 weeks ago 
Hi Anvesh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on a0ae2a256046c0c5d3778d1a194ff2e171f16e5f]

url:    https://github.com/intel-lab-lkp/linux/commits/Anvesh-Jain-P/dt-bindings-embedded-controller-Add-EC-bindings-for-Qualcomm-reference-devices/20260314-163112
base:   a0ae2a256046c0c5d3778d1a194ff2e171f16e5f
patch link:    https://lore.kernel.org/r/20260313-v04-add-driver-for-ec-v4-2-ca9d0efd62aa%40oss.qualcomm.com
patch subject: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20260317/202603170132.IivRdk9p-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project f46a5153850c1303d687233d4adf699b01041da8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260317/202603170132.IivRdk9p-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603170132.IivRdk9p-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/platform/arm64/qcom-hamoa-ec.c:395:31: warning: variable 'cdev' is uninitialized when used here [-Wuninitialized]
     395 |                         dev_err_probe(dev, PTR_ERR(cdev),
         |                                                    ^~~~
   drivers/platform/arm64/qcom-hamoa-ec.c:350:34: note: initialize the variable 'cdev' to silence this warning
     350 |         struct qcom_ec_cooling_dev *cdev;
         |                                         ^
         |                                          = NULL
   1 warning generated.


vim +/cdev +395 drivers/platform/arm64/qcom-hamoa-ec.c

   347	
   348	static int qcom_ec_probe(struct i2c_client *client)
   349	{
   350		struct qcom_ec_cooling_dev *cdev;
   351		struct device *dev = &client->dev;
   352		struct qcom_ec *ec;
   353		int ret, i;
   354	
   355		ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
   356		if (!ec)
   357			return -ENOMEM;
   358	
   359		ec->client = client;
   360	
   361		ret = devm_request_threaded_irq(dev, client->irq, NULL, qcom_ec_irq,
   362						IRQF_ONESHOT, "qcom_ec", ec);
   363		if (ret < 0)
   364			return dev_err_probe(dev, ret, "Failed to get irq\n");
   365	
   366		i2c_set_clientdata(client, ec);
   367	
   368		ret = qcom_ec_read_fw_version(dev);
   369		if (ret < 0)
   370			return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");
   371	
   372		ret = qcom_ec_thermal_capabilities(dev);
   373		if (ret < 0)
   374			return dev_err_probe(dev, ret, "Failed to read thermal capabilities\n");
   375	
   376		ret = qcom_ec_sci_evt_control(dev, true);
   377		if (ret < 0)
   378			return dev_err_probe(dev, ret, "Failed to enable SCI events\n");
   379	
   380		ec->ec_cdev = devm_kcalloc(dev, ec->thermal_cap.fan_cnt, sizeof(*ec->ec_cdev), GFP_KERNEL);
   381		if (!ec->ec_cdev)
   382			return -ENOMEM;
   383	
   384		for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
   385			struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
   386			char name[EC_FAN_NAME_SIZE];
   387	
   388			snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
   389			ec_cdev->fan_id = i + 1;
   390			ec_cdev->parent_dev = dev;
   391	
   392			ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
   393									&qcom_ec_thermal_ops);
   394			if (IS_ERR(ec_cdev->cdev)) {
 > 395				dev_err_probe(dev, PTR_ERR(cdev),
   396					      "Thermal cooling device registration failed\n");
   397				ret = -EINVAL;
   398				goto unroll_cooling_dev;
   399			}
   400		}
   401	
   402		return 0;
   403	
   404	unroll_cooling_dev:
   405		for (i--; i >= 0; i--) {
   406			struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
   407	
   408			if (ec_cdev->cdev) {
   409				thermal_cooling_device_unregister(ec_cdev->cdev);
   410				ec_cdev->cdev = NULL;
   411			}
   412		}
   413	
   414		return ret;
   415	}
   416	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by kernel test robot 3 weeks ago 
Hi Anvesh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on a0ae2a256046c0c5d3778d1a194ff2e171f16e5f]

url:    https://github.com/intel-lab-lkp/linux/commits/Anvesh-Jain-P/dt-bindings-embedded-controller-Add-EC-bindings-for-Qualcomm-reference-devices/20260314-163112
base:   a0ae2a256046c0c5d3778d1a194ff2e171f16e5f
patch link:    https://lore.kernel.org/r/20260313-v04-add-driver-for-ec-v4-2-ca9d0efd62aa%40oss.qualcomm.com
patch subject: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
config: csky-allmodconfig (https://download.01.org/0day-ci/archive/20260316/202603162107.pwsms8tn-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260316/202603162107.pwsms8tn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603162107.pwsms8tn-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/platform/arm64/qcom-hamoa-ec.c: In function 'qcom_ec_probe':
>> drivers/platform/arm64/qcom-hamoa-ec.c:388:63: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 8 [-Wformat-truncation=]
     388 |                 snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
         |                                                               ^~
   drivers/platform/arm64/qcom-hamoa-ec.c:388:50: note: directive argument in the range [-2147483641, 254]
     388 |                 snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
         |                                                  ^~~~~~~~~~~~~~~~
   drivers/platform/arm64/qcom-hamoa-ec.c:388:17: note: 'snprintf' output between 14 and 24 bytes into a destination of size 20
     388 |                 snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +388 drivers/platform/arm64/qcom-hamoa-ec.c

   347	
   348	static int qcom_ec_probe(struct i2c_client *client)
   349	{
   350		struct qcom_ec_cooling_dev *cdev;
   351		struct device *dev = &client->dev;
   352		struct qcom_ec *ec;
   353		int ret, i;
   354	
   355		ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
   356		if (!ec)
   357			return -ENOMEM;
   358	
   359		ec->client = client;
   360	
   361		ret = devm_request_threaded_irq(dev, client->irq, NULL, qcom_ec_irq,
   362						IRQF_ONESHOT, "qcom_ec", ec);
   363		if (ret < 0)
   364			return dev_err_probe(dev, ret, "Failed to get irq\n");
   365	
   366		i2c_set_clientdata(client, ec);
   367	
   368		ret = qcom_ec_read_fw_version(dev);
   369		if (ret < 0)
   370			return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");
   371	
   372		ret = qcom_ec_thermal_capabilities(dev);
   373		if (ret < 0)
   374			return dev_err_probe(dev, ret, "Failed to read thermal capabilities\n");
   375	
   376		ret = qcom_ec_sci_evt_control(dev, true);
   377		if (ret < 0)
   378			return dev_err_probe(dev, ret, "Failed to enable SCI events\n");
   379	
   380		ec->ec_cdev = devm_kcalloc(dev, ec->thermal_cap.fan_cnt, sizeof(*ec->ec_cdev), GFP_KERNEL);
   381		if (!ec->ec_cdev)
   382			return -ENOMEM;
   383	
   384		for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
   385			struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
   386			char name[EC_FAN_NAME_SIZE];
   387	
 > 388			snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
   389			ec_cdev->fan_id = i + 1;
   390			ec_cdev->parent_dev = dev;
   391	
   392			ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
   393									&qcom_ec_thermal_ops);
   394			if (IS_ERR(ec_cdev->cdev)) {
   395				dev_err_probe(dev, PTR_ERR(cdev),
   396					      "Thermal cooling device registration failed\n");
   397				ret = -EINVAL;
   398				goto unroll_cooling_dev;
   399			}
   400		}
   401	
   402		return 0;
   403	
   404	unroll_cooling_dev:
   405		for (i--; i >= 0; i--) {
   406			struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
   407	
   408			if (ec_cdev->cdev) {
   409				thermal_cooling_device_unregister(ec_cdev->cdev);
   410				ec_cdev->cdev = NULL;
   411			}
   412		}
   413	
   414		return ret;
   415	}
   416	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Konrad Dybcio 3 weeks, 1 day ago 
On 3/13/26 11:29 AM, Anvesh Jain P wrote:
> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> 
> Add Embedded controller driver support for Hamoa/Purwa/Glymur qualcomm
> reference boards. It handles fan control, temperature sensors, access
> to EC state changes and supports reporting suspend entry/exit to the
> EC.
> 
> Co-developed-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> Co-developed-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> ---

[...]

> + * ------------------------------------------------------------------------------
> + * | Offset		| Name		| Description				|
> + * ------------------------------------------------------------------------------
> + * | 0x00		| Byte count	| Number of bytes in response		|
> + * |			|		| (exluding byte count)			|
> + * ------------------------------------------------------------------------------
> + * | 0x02 (LSB)	| EC Thermal	| Bit 0-1: Number of fans		|

qcom_ec_fan_debug_mode_off() documentation only mentions values 1/2
to be correct - is that the case?

> + * | 0x3		| Capabilities	| Bit 2-4: Type of fan			|

What types are there? CPU/AUX or similar?

Konrad
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Anvesh Jain P 3 weeks ago 

On 3/16/2026 3:59 PM, Konrad Dybcio wrote:
>> + * | Offset		| Name		| Description				|
>> + * ------------------------------------------------------------------------------
>> + * | 0x00		| Byte count	| Number of bytes in response		|
>> + * |			|		| (exluding byte count)			|
>> + * ------------------------------------------------------------------------------
>> + * | 0x02 (LSB)	| EC Thermal	| Bit 0-1: Number of fans		|
> qcom_ec_fan_debug_mode_off() documentation only mentions values 1/2
> to be correct - is that the case?
> 

Yes, qcom_ec_fan_debug_mode_off() will only support values 1/2. The
above documentation mentions about number of fans available, max being 2.

>> + * | 0x3		| Capabilities	| Bit 2-4: Type of fan			|
> What types are there? CPU/AUX or similar?
> 

This is fan vendor identification byte. 0 being invalid/not present.

-- 
Best Regards,
Anvesh
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Konrad Dybcio 3 weeks, 1 day ago 
On 3/13/26 11:29 AM, Anvesh Jain P wrote:
> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> 
> Add Embedded controller driver support for Hamoa/Purwa/Glymur qualcomm
> reference boards. It handles fan control, temperature sensors, access
> to EC state changes and supports reporting suspend entry/exit to the
> EC.
> 
> Co-developed-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> Co-developed-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> ---

[...]

> + * ------------------------------------------------------------------------------
> + * | Offset		| Name		| Description				|
> + * ------------------------------------------------------------------------------
> + * | 0x00		| Byte count	| Number of bytes in response		|
> + * |			|		| (exluding byte count)			|
> + * ------------------------------------------------------------------------------
> + * | 0x02 (LSB)	| EC Thermal	| Bit 0-1: Number of fans		|
> + * | 0x3		| Capabilities	| Bit 2-4: Type of fan			|
> + * |			|		| Bit 5-6: Reserved			|
> + * |			|		| Bit 7: Data Valid/Invalid		|
> + * |			|		|	 (Valid - 1, Invalid - 0)	|
> + * |			|		| Bit 8-15: Thermistor 0 - 7 presence	|
> + * |			|		|	    (0 present, 1 absent)	|
						     ^ huh??

I see that it's not currently used, but I think flipping these
bits would make it easier to comprehend down the line

[...]

> +	default:
> +		dev_dbg(dev, "Unknown EC event: %d\n", val);

Maybe dev_notice(), this would be good to log

> +		break;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int qcom_ec_sci_evt_control(struct device *dev, bool enable)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 control = enable ? 1 : 0;
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(client, EC_SCI_EVT_CONTROL_CMD, control);
> +
> +	return ret;

return i2c_smbus_write_byte_data(client, EC_SCI_EVT_CONTROL_CMD, !!enable);

[...]

> +static int qcom_ec_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(client, EC_MODERN_STANDBY_CMD, EC_MODERN_STANDBY_ENTER);
> +
> +	return ret;

In a pattern like this, unless you have some error checking/logging to
do, you can just return directly

[...]

> +	ret = qcom_ec_read_fw_version(dev);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");

"EC"

[...]

> +	for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
> +		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
> +		char name[EC_FAN_NAME_SIZE];
> +
> +		snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
> +		ec_cdev->fan_id = i + 1;
> +		ec_cdev->parent_dev = dev;
> +
> +		ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
> +								&qcom_ec_thermal_ops);
> +		if (IS_ERR(ec_cdev->cdev)) {
> +			dev_err_probe(dev, PTR_ERR(cdev),
> +				      "Thermal cooling device registration failed\n");
> +			ret = -EINVAL;
> +			goto unroll_cooling_dev;
> +		}
> +	}
> +
> +	return 0;
> +
> +unroll_cooling_dev:

For those cases, one would usually add add a devres-managed version of
the helper (devm_foo_bar()), removing the need to repeat this in the 
driver remove callback as well

Konrad
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Anvesh Jain P 3 weeks ago 

On 3/16/2026 3:57 PM, Konrad Dybcio wrote:
> On 3/13/26 11:29 AM, Anvesh Jain P wrote:
>> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>
>> Add Embedded controller driver support for Hamoa/Purwa/Glymur qualcomm
>> reference boards. It handles fan control, temperature sensors, access
>> to EC state changes and supports reporting suspend entry/exit to the
>> EC.
>>
>> Co-developed-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> Co-developed-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
>> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> + * ------------------------------------------------------------------------------
>> + * | Offset		| Name		| Description				|
>> + * ------------------------------------------------------------------------------
>> + * | 0x00		| Byte count	| Number of bytes in response		|
>> + * |			|		| (exluding byte count)			|
>> + * ------------------------------------------------------------------------------
>> + * | 0x02 (LSB)	| EC Thermal	| Bit 0-1: Number of fans		|
>> + * | 0x3		| Capabilities	| Bit 2-4: Type of fan			|
>> + * |			|		| Bit 5-6: Reserved			|
>> + * |			|		| Bit 7: Data Valid/Invalid		|
>> + * |			|		|	 (Valid - 1, Invalid - 0)	|
>> + * |			|		| Bit 8-15: Thermistor 0 - 7 presence	|
>> + * |			|		|	    (0 present, 1 absent)	|
> 						     ^ huh??
> 
> I see that it's not currently used, but I think flipping these
> bits would make it easier to comprehend down the line
> 
> [...]
> 

Thanks for taking time to review the series

The 0=present encoding was taken directly from the EC firmware spec, but
on closer inspection this appears to be a typo in the spec — the correct
behavior is 1=present, 0=absent. Will update the comment in v5.

>> +	default:
>> +		dev_dbg(dev, "Unknown EC event: %d\n", val);
> 
> Maybe dev_notice(), this would be good to log
>

Agreed, will switch to dev_notice for unknown EC events in v5.

>> +		break;
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int qcom_ec_sci_evt_control(struct device *dev, bool enable)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	u8 control = enable ? 1 : 0;
>> +	int ret;
>> +
>> +	ret = i2c_smbus_write_byte_data(client, EC_SCI_EVT_CONTROL_CMD, control);
>> +
>> +	return ret;
> 
> return i2c_smbus_write_byte_data(client, EC_SCI_EVT_CONTROL_CMD, !!enable);
> 
> [...]
> 

Agreed, will simplify in v5

>> +static int qcom_ec_resume(struct device *dev)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	int ret;
>> +
>> +	ret = i2c_smbus_write_byte_data(client, EC_MODERN_STANDBY_CMD, EC_MODERN_STANDBY_ENTER);
>> +
>> +	return ret;
> 
> In a pattern like this, unless you have some error checking/logging to
> do, you can just return directly
> 
> [...]
> 

Agreed, will simplify in v5.

>> +	ret = qcom_ec_read_fw_version(dev);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");
> 
> "EC"
> 
> [...]
> 

Agreed, will fix in v5

>> +	for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
>> +		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
>> +		char name[EC_FAN_NAME_SIZE];
>> +
>> +		snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
>> +		ec_cdev->fan_id = i + 1;
>> +		ec_cdev->parent_dev = dev;
>> +
>> +		ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
>> +								&qcom_ec_thermal_ops);
>> +		if (IS_ERR(ec_cdev->cdev)) {
>> +			dev_err_probe(dev, PTR_ERR(cdev),
>> +				      "Thermal cooling device registration failed\n");
>> +			ret = -EINVAL;
>> +			goto unroll_cooling_dev;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +
>> +unroll_cooling_dev:
> 
> For those cases, one would usually add add a devres-managed version of
> the helper (devm_foo_bar()), removing the need to repeat this in the 
> driver remove callback as well
> 
> Konrad
Agreed, will switch to devm_thermal_of_cooling_device_register in v5

-- 
Best Regards,
Anvesh
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Krzysztof Kozlowski 3 weeks, 3 days ago 
On 13/03/2026 11:29, Anvesh Jain P wrote:
> +
> +static irqreturn_t qcom_ec_irq(int irq, void *data)
> +{
> +	struct qcom_ec *ec = data;
> +	struct device *dev = &ec->client->dev;
> +	int val;
> +
> +	val = i2c_smbus_read_byte_data(ec->client, EC_SCI_EVT_READ_CMD);
> +	if (val < 0) {
> +		dev_err(dev, "Failed to read EC SCI Event: %d\n", val);

ratelimit

> +		return IRQ_HANDLED;
> +	}
> +
> +	switch (val) {
> +	case EC_FAN1_STATUS_CHANGE_EVT:
> +		dev_dbg(dev, "Fan1 status changed\n");

ratelimit everywhere further. You are in interrupt handler so imagine
same interrupt keep happening because of constant overheat.

> +		break;
> +	case EC_FAN2_STATUS_CHANGE_EVT:
> +		dev_dbg(dev, "Fan2 status changed\n");
> +		break;
> +	case EC_FAN1_SPEED_CHANGE_EVT:
> +		dev_dbg(dev, "Fan1 speed crossed low/high trip point\n");
> +		break;
> +	case EC_FAN2_SPEED_CHANGE_EVT:
> +		dev_dbg(dev, "Fan2 speed crossed low/high trip point\n");
> +		break;
> +	case EC_NEW_LUT_SET_EVT:
> +		dev_dbg(dev, "New LUT set\n");
> +		break;
> +	case EC_FAN_PROFILE_SWITCH_EVT:
> +		dev_dbg(dev, "FAN Profile switched\n");
> +		break;
> +	case EC_THERMISTOR_1_THRESHOLD_CROSS_EVT:
> +		dev_dbg(dev, "Thermistor 1 threshold crossed\n");
> +		break;
> +	case EC_THERMISTOR_2_THRESHOLD_CROSS_EVT:
> +		dev_dbg(dev, "Thermistor 2 threshold crossed\n");
> +		break;
> +	case EC_THERMISTOR_3_THRESHOLD_CROSS_EVT:
> +		dev_dbg(dev, "Thermistor 3 threshold crossed\n");
> +		break;
> +	case EC_RECOVERED_FROM_RESET_EVT:
> +		dev_dbg(dev, "EC recovered from reset\n");
> +		break;
> +	default:
> +		dev_dbg(dev, "Unknown EC event: %d\n", val);
> +		break;
> +	}
> +


...

> +
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL, qcom_ec_irq,
> +					IRQF_ONESHOT, "qcom_ec", ec);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get irq\n");

No need for message, just return ret.

> +
> +	i2c_set_clientdata(client, ec);
> +
> +	ret = qcom_ec_read_fw_version(dev);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");
> +
> +	ret = qcom_ec_thermal_capabilities(dev);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to read thermal capabilities\n");
> +
> +	ret = qcom_ec_sci_evt_control(dev, true);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to enable SCI events\n");
> +
> +	ec->ec_cdev = devm_kcalloc(dev, ec->thermal_cap.fan_cnt, sizeof(*ec->ec_cdev), GFP_KERNEL);
> +	if (!ec->ec_cdev)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
> +		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
> +		char name[EC_FAN_NAME_SIZE];
> +
> +		snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
> +		ec_cdev->fan_id = i + 1;
> +		ec_cdev->parent_dev = dev;
> +
> +		ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
> +								&qcom_ec_thermal_ops);
> +		if (IS_ERR(ec_cdev->cdev)) {
> +			dev_err_probe(dev, PTR_ERR(cdev),
> +				      "Thermal cooling device registration failed\n");
> +			ret = -EINVAL;

Why do you override actual return code?

> +			goto unroll_cooling_dev;
> +		}
> +	}
> +
> +	return 0;
> +
> +unroll_cooling_dev:
> +	for (i--; i >= 0; i--) {
> +		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
> +
> +		if (ec_cdev->cdev) {
> +			thermal_cooling_device_unregister(ec_cdev->cdev);
> +			ec_cdev->cdev = NULL;
> +		}
> +	}
> +
> +	return ret;
> +}


Best regards,
Krzysztof
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Anvesh Jain P 3 weeks, 1 day ago 

On 3/14/2026 12:35 AM, Krzysztof Kozlowski wrote:
> On 13/03/2026 11:29, Anvesh Jain P wrote:
>> +
>> +static irqreturn_t qcom_ec_irq(int irq, void *data)
>> +{
>> +	struct qcom_ec *ec = data;
>> +	struct device *dev = &ec->client->dev;
>> +	int val;
>> +
>> +	val = i2c_smbus_read_byte_data(ec->client, EC_SCI_EVT_READ_CMD);
>> +	if (val < 0) {
>> +		dev_err(dev, "Failed to read EC SCI Event: %d\n", val);
> 
> ratelimit
> 
>> +		return IRQ_HANDLED;
>> +	}
>> +
>> +	switch (val) {
>> +	case EC_FAN1_STATUS_CHANGE_EVT:
>> +		dev_dbg(dev, "Fan1 status changed\n");
> 
> ratelimit everywhere further. You are in interrupt handler so imagine
> same interrupt keep happening because of constant overheat.
> 

Agreed. I will switch to dev_err_ratelimited() and dev_dbg_ratelimited()
throughout the IRQ handler to prevent log spam during potential
interrupt storms.

>> +		break;
>> +	case EC_FAN2_STATUS_CHANGE_EVT:
>> +		dev_dbg(dev, "Fan2 status changed\n");
>> +		break;
>> +	case EC_FAN1_SPEED_CHANGE_EVT:
>> +		dev_dbg(dev, "Fan1 speed crossed low/high trip point\n");
>> +		break;
>> +	case EC_FAN2_SPEED_CHANGE_EVT:
>> +		dev_dbg(dev, "Fan2 speed crossed low/high trip point\n");
>> +		break;
>> +	case EC_NEW_LUT_SET_EVT:
>> +		dev_dbg(dev, "New LUT set\n");
>> +		break;
>> +	case EC_FAN_PROFILE_SWITCH_EVT:
>> +		dev_dbg(dev, "FAN Profile switched\n");
>> +		break;
>> +	case EC_THERMISTOR_1_THRESHOLD_CROSS_EVT:
>> +		dev_dbg(dev, "Thermistor 1 threshold crossed\n");
>> +		break;
>> +	case EC_THERMISTOR_2_THRESHOLD_CROSS_EVT:
>> +		dev_dbg(dev, "Thermistor 2 threshold crossed\n");
>> +		break;
>> +	case EC_THERMISTOR_3_THRESHOLD_CROSS_EVT:
>> +		dev_dbg(dev, "Thermistor 3 threshold crossed\n");
>> +		break;
>> +	case EC_RECOVERED_FROM_RESET_EVT:
>> +		dev_dbg(dev, "EC recovered from reset\n");
>> +		break;
>> +	default:
>> +		dev_dbg(dev, "Unknown EC event: %d\n", val);
>> +		break;
>> +	}
>> +
> 
> 
> ...
> 
>> +
>> +	ret = devm_request_threaded_irq(dev, client->irq, NULL, qcom_ec_irq,
>> +					IRQF_ONESHOT, "qcom_ec", ec);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to get irq\n");
> 
> No need for message, just return ret.
> 

Will do. I'll drop the message and just return ret.

>> +
>> +	i2c_set_clientdata(client, ec);
>> +
>> +	ret = qcom_ec_read_fw_version(dev);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to read ec firmware version\n");
>> +
>> +	ret = qcom_ec_thermal_capabilities(dev);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to read thermal capabilities\n");
>> +
>> +	ret = qcom_ec_sci_evt_control(dev, true);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to enable SCI events\n");
>> +
>> +	ec->ec_cdev = devm_kcalloc(dev, ec->thermal_cap.fan_cnt, sizeof(*ec->ec_cdev), GFP_KERNEL);
>> +	if (!ec->ec_cdev)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < ec->thermal_cap.fan_cnt; i++) {
>> +		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
>> +		char name[EC_FAN_NAME_SIZE];
>> +
>> +		snprintf(name, EC_FAN_NAME_SIZE, "qcom_ec_fan_%d", i);
>> +		ec_cdev->fan_id = i + 1;
>> +		ec_cdev->parent_dev = dev;
>> +
>> +		ec_cdev->cdev = thermal_cooling_device_register(name, ec_cdev,
>> +								&qcom_ec_thermal_ops);
>> +		if (IS_ERR(ec_cdev->cdev)) {
>> +			dev_err_probe(dev, PTR_ERR(cdev),
>> +				      "Thermal cooling device registration failed\n");
>> +			ret = -EINVAL;
> 
> Why do you override actual return code?
> 

 I'll fix this to propagate the actual error code.

>> +			goto unroll_cooling_dev;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +
>> +unroll_cooling_dev:
>> +	for (i--; i >= 0; i--) {
>> +		struct qcom_ec_cooling_dev *ec_cdev = &ec->ec_cdev[i];
>> +
>> +		if (ec_cdev->cdev) {
>> +			thermal_cooling_device_unregister(ec_cdev->cdev);
>> +			ec_cdev->cdev = NULL;
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
> 
> 
> Best regards,
> Krzysztof

Thanks for the review.

-- 
Best Regards,
Anvesh
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Dmitry Baryshkov 3 weeks, 3 days ago 
On Fri, Mar 13, 2026 at 03:59:52PM +0530, Anvesh Jain P wrote:
> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> 
> Add Embedded controller driver support for Hamoa/Purwa/Glymur qualcomm
> reference boards. It handles fan control, temperature sensors, access
> to EC state changes and supports reporting suspend entry/exit to the
> EC.
> 
> Co-developed-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> Co-developed-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> ---
>  MAINTAINERS                            |   8 +
>  drivers/platform/arm64/Kconfig         |  12 +
>  drivers/platform/arm64/Makefile        |   1 +
>  drivers/platform/arm64/qcom-hamoa-ec.c | 468 +++++++++++++++++++++++++++++++++
>  4 files changed, 489 insertions(+)
> 
> --- /dev/null
> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
> @@ -0,0 +1,468 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024 Maya Matuszczyk <maccraft123mc@gmail.com>
> + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.

Drop the year from Qualcomm copyright, please.

> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/thermal.h>

Please add <linux/slab.h> and <linux/interrupt.h>

With that and with the typo fixed,


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>



-- 
With best wishes
Dmitry
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Anvesh Jain P 3 weeks, 3 days ago 

On 3/13/2026 8:05 PM, Dmitry Baryshkov wrote:
> On Fri, Mar 13, 2026 at 03:59:52PM +0530, Anvesh Jain P wrote:
>> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>
>> Add Embedded controller driver support for Hamoa/Purwa/Glymur qualcomm
>> reference boards. It handles fan control, temperature sensors, access
>> to EC state changes and supports reporting suspend entry/exit to the
>> EC.
>>
>> Co-developed-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> Co-developed-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
>> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
>> ---
>>  MAINTAINERS                            |   8 +
>>  drivers/platform/arm64/Kconfig         |  12 +
>>  drivers/platform/arm64/Makefile        |   1 +
>>  drivers/platform/arm64/qcom-hamoa-ec.c | 468 +++++++++++++++++++++++++++++++++
>>  4 files changed, 489 insertions(+)
>>
>> --- /dev/null
>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>> @@ -0,0 +1,468 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2024 Maya Matuszczyk <maccraft123mc@gmail.com>
>> + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
> 
> Drop the year from Qualcomm copyright, please.

Will fix in v5.

> 
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pm.h>
>> +#include <linux/thermal.h>
> 
> Please add <linux/slab.h> and <linux/interrupt.h>

Will add both headers in v5.

> 
> With that and with the typo fixed,

Thanks for the review!

> 
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 
> 
> 

-- 
Best Regards,
Anvesh
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Bryan O'Donoghue 3 weeks, 3 days ago 
On 13/03/2026 10:29, Anvesh Jain P wrote:
> + * Read Response:
> + * ----------------------------------------------------------------------
> + * | Offset	| Name		| Description				|
> + * ----------------------------------------------------------------------
> + * | 0x00	| Byte count	| Number of bytes in response		|
> + * |		|		| (exluding byte count)			|

Great to see this documented in the code.

Please run checkpatch and take heed of what it says.

0002-platform-arm64-Add-driver-for-EC-found-on-Qualcomm-r.patch has no 
obvious style problems and is ready for submission.
0002-platform-arm64-Add-driver-for-EC-found-on-Qualcomm-r.patch:184: 
exluding ==> excluding, exuding
0002-platform-arm64-Add-driver-for-EC-found-on-Qualcomm-r.patch:224: 
exluding ==> excluding, exuding
total: 0 errors, 0 warnings, 0 checks, 28 lines checked

---
bod
Re: [PATCH v4 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
Posted by Anvesh Jain P 3 weeks, 3 days ago 

On 3/13/2026 5:47 PM, Bryan O'Donoghue wrote:
> On 13/03/2026 10:29, Anvesh Jain P wrote:
>> + * Read Response:
>> + *
>> ----------------------------------------------------------------------
>> + * | Offset    | Name        | Description                |
>> + *
>> ----------------------------------------------------------------------
>> + * | 0x00    | Byte count    | Number of bytes in response        |
>> + * |        |        | (exluding byte count)            |
> 
> Great to see this documented in the code.
> 
> Please run checkpatch and take heed of what it says.
> 
> 0002-platform-arm64-Add-driver-for-EC-found-on-Qualcomm-r.patch has no
> obvious style problems and is ready for submission.
> 0002-platform-arm64-Add-driver-for-EC-found-on-Qualcomm-r.patch:184:
> exluding ==> excluding, exuding
> 0002-platform-arm64-Add-driver-for-EC-found-on-Qualcomm-r.patch:224:
> exluding ==> excluding, exuding
> total: 0 errors, 0 warnings, 0 checks, 28 lines checked

Thanks for the review! Confirmed the typo — will fix both occurrences in
the next revision of the patch series.

> 
> ---
> bod

Best regards,
Anvesh

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.