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>
---
 MAINTAINERS                            |   7 +
 drivers/platform/arm64/Kconfig         |  12 +
 drivers/platform/arm64/Makefile        |   1 +
 drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
 4 files changed, 482 insertions(+)
 create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2882a67bdf6d..dc72093375ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21932,6 +21932,13 @@ 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>
+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..83aa869fad8f
--- /dev/null
+++ b/drivers/platform/arm64/qcom-hamoa-ec.c
@@ -0,0 +1,462 @@
+// 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/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_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;
+	/* EC bus transaction lock */
+	struct mutex lock;
+};
+
+static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
+{
+	int ret;
+
+	mutex_lock(&ec->lock);
+	ret = i2c_smbus_read_i2c_block_data(ec->client, cmd, resp_len, resp);
+	mutex_unlock(&ec->lock);
+	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 = max(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;
+
+	mutex_lock(&ec->lock);
+	val = i2c_smbus_read_byte_data(ec->client, EC_SCI_EVT_READ_CMD);
+	mutex_unlock(&ec->lock);
+	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);
+	struct qcom_ec *ec = i2c_get_clientdata(client);
+	u8 control = enable ? 1 : 0;
+	int ret;
+
+	mutex_lock(&ec->lock);
+	ret = i2c_smbus_write_byte_data(client, EC_SCI_EVT_CONTROL_CMD, control);
+	mutex_unlock(&ec->lock);
+
+	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_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);
+	struct qcom_ec *ec = i2c_get_clientdata(client);
+	int ret;
+
+	mutex_lock(&ec->lock);
+	ret = i2c_smbus_write_byte_data(client, EC_MODERN_STANDBY_CMD, EC_MODERN_STANDBY_ENTER);
+	mutex_unlock(&ec->lock);
+
+	return ret;
+}
+
+static int qcom_ec_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct qcom_ec *ec = i2c_get_clientdata(client);
+	int ret;
+
+	mutex_lock(&ec->lock);
+	ret = i2c_smbus_write_byte_data(client, EC_MODERN_STANDBY_CMD, EC_MODERN_STANDBY_EXIT);
+	mutex_unlock(&ec->lock);
+
+	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;
+
+	mutex_init(&ec->lock);
+	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];
+
+		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-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

Hi Sibi,

kernel test robot noticed the following build errors:

[auto build test ERROR on a0ae2a256046c0c5d3778d1a194ff2e171f16e5f]

url:    https://github.com/intel-lab-lkp/linux/commits/Sibi-Sankar/dt-bindings-embedded-controller-Add-EC-bindings-for-Qualcomm-reference-devices/20260309-074148
base:   a0ae2a256046c0c5d3778d1a194ff2e171f16e5f
patch link:    https://lore.kernel.org/r/20260308233646.2318676-3-sibi.sankar%40oss.qualcomm.com
patch subject: [PATCH V3 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20260310/202603101413.1egT3NBN-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260310/202603101413.1egT3NBN-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/202603101413.1egT3NBN-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/ioport.h:16,
                    from include/linux/acpi.h:13,
                    from include/linux/i2c.h:13,
                    from drivers/platform/arm64/qcom-hamoa-ec.c:7:
   drivers/platform/arm64/qcom-hamoa-ec.c: In function 'qcom_ec_thermal_capabilities':
>> drivers/platform/arm64/qcom-hamoa-ec.c:56:42: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
      56 | #define EC_THERMAL_FAN_CNT(x)           (FIELD_GET(GENMASK(1, 0), (x)))
         |                                          ^~~~~~~~~
   include/linux/minmax.h:92:35: note: in definition of macro '__careful_cmp_once'
      92 |         auto ux = (x); auto uy = (y);                   \
         |                                   ^
   include/linux/minmax.h:112:25: note: in expansion of macro '__careful_cmp'
     112 | #define max(x, y)       __careful_cmp(max, x, y)
         |                         ^~~~~~~~~~~~~
   drivers/platform/arm64/qcom-hamoa-ec.c:170:24: note: in expansion of macro 'max'
     170 |         cap->fan_cnt = max(EC_MAX_FAN_CNT, EC_THERMAL_FAN_CNT(resp[1]));
         |                        ^~~
   drivers/platform/arm64/qcom-hamoa-ec.c:170:44: note: in expansion of macro 'EC_THERMAL_FAN_CNT'
     170 |         cap->fan_cnt = max(EC_MAX_FAN_CNT, EC_THERMAL_FAN_CNT(resp[1]));
         |                                            ^~~~~~~~~~~~~~~~~~


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

   > 7	#include <linux/i2c.h>
     8	#include <linux/kernel.h>
     9	#include <linux/module.h>
    10	#include <linux/pm.h>
    11	#include <linux/thermal.h>
    12	
    13	#define EC_SCI_EVT_READ_CMD	0x05
    14	#define EC_FW_VERSION_CMD	0x0e
    15	#define EC_MODERN_STANDBY_CMD	0x23
    16	#define EC_FAN_DBG_CONTROL_CMD	0x30
    17	#define EC_SCI_EVT_CONTROL_CMD	0x35
    18	#define EC_THERMAL_CAP_CMD	0x42
    19	
    20	#define EC_FW_VERSION_RESP_LEN	4
    21	#define EC_THERMAL_CAP_RESP_LEN	3
    22	#define EC_FAN_DEBUG_CMD_LEN	6
    23	#define EC_FAN_SPEED_DATA_SIZE	4
    24	
    25	#define EC_MODERN_STANDBY_ENTER	0x01
    26	#define EC_MODERN_STANDBY_EXIT	0x00
    27	
    28	#define EC_FAN_DEBUG_MODE_ON    BIT(0)
    29	#define EC_FAN_ON               BIT(1)
    30	#define EC_FAN_DEBUG_TYPE_PWM   BIT(2)
    31	#define EC_MAX_FAN_CNT		2
    32	#define EC_FAN_NAME_SIZE	20
    33	#define EC_FAN_MAX_PWM		255
    34	
    35	enum qcom_ec_sci_events {
    36		EC_FAN1_STATUS_CHANGE_EVT = 0x30,
    37		EC_FAN2_STATUS_CHANGE_EVT,
    38		EC_FAN1_SPEED_CHANGE_EVT,
    39		EC_FAN2_SPEED_CHANGE_EVT,
    40		EC_NEW_LUT_SET_EVT,
    41		EC_FAN_PROFILE_SWITCH_EVT,
    42		EC_THERMISTOR_1_THRESHOLD_CROSS_EVT,
    43		EC_THERMISTOR_2_THRESHOLD_CROSS_EVT,
    44		EC_THERMISTOR_3_THRESHOLD_CROSS_EVT,
    45		/* Reserved: 0x39 - 0x3c/0x3f */
    46		EC_RECOVERED_FROM_RESET_EVT = 0x3d,
    47	};
    48	
    49	struct qcom_ec_version {
    50		u8 main_version;
    51		u8 sub_version;
    52		u8 test_version;
    53	};
    54	
    55	struct qcom_ec_thermal_cap {
  > 56	#define EC_THERMAL_FAN_CNT(x)		(FIELD_GET(GENMASK(1, 0), (x)))
    57	#define EC_THERMAL_FAN_TYPE(x)		(FIELD_GET(GENMASK(4, 2), (x)))
    58	#define EC_THERMAL_THERMISTOR_MASK(x)	(FIELD_GET(GENMASK(7, 0), (x)))
    59		u8 fan_cnt;
    60		u8 fan_type;
    61		u8 thermistor_mask;
    62	};
    63	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Hi Sibi,

kernel test robot noticed the following build errors:

[auto build test ERROR on a0ae2a256046c0c5d3778d1a194ff2e171f16e5f]

url:    https://github.com/intel-lab-lkp/linux/commits/Sibi-Sankar/dt-bindings-embedded-controller-Add-EC-bindings-for-Qualcomm-reference-devices/20260309-074148
base:   a0ae2a256046c0c5d3778d1a194ff2e171f16e5f
patch link:    https://lore.kernel.org/r/20260308233646.2318676-3-sibi.sankar%40oss.qualcomm.com
patch subject: [PATCH V3 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260310/202603101317.ZR8NjiRC-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260310/202603101317.ZR8NjiRC-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/202603101317.ZR8NjiRC-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/platform/arm64/qcom-hamoa-ec.c:170:37: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     170 |         cap->fan_cnt = max(EC_MAX_FAN_CNT, EC_THERMAL_FAN_CNT(resp[1]));
         |                                            ^
   drivers/platform/arm64/qcom-hamoa-ec.c:56:33: note: expanded from macro 'EC_THERMAL_FAN_CNT'
      56 | #define EC_THERMAL_FAN_CNT(x)           (FIELD_GET(GENMASK(1, 0), (x)))
         |                                          ^
   drivers/platform/arm64/qcom-hamoa-ec.c:171:18: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     171 |         cap->fan_type = EC_THERMAL_FAN_TYPE(resp[1]);
         |                         ^
   drivers/platform/arm64/qcom-hamoa-ec.c:57:34: note: expanded from macro 'EC_THERMAL_FAN_TYPE'
      57 | #define EC_THERMAL_FAN_TYPE(x)          (FIELD_GET(GENMASK(4, 2), (x)))
         |                                          ^
   2 errors generated.


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

   137	
   138	/*
   139	 * EC Device Thermal Capabilities:
   140	 *
   141	 * Read Response:
   142	 * ----------------------------------------------------------------------
   143	 * | Offset	| Name		| Description				|
   144	 * ----------------------------------------------------------------------
   145	 * | 0x00	| Byte count	| Number of bytes in response		|
   146	 * |		|		| (exluding byte count)			|
   147	 * ----------------------------------------------------------------------
   148	 * | 0x02 (LSB)	| EC Thermal	| Bit 0-1: Number of fans		|
   149	 * | 0x3	| Capabilities	| Bit 2-4: Type of fan			|
   150	 * |		|		| Bit 5-6: Reserved			|
   151	 * |		|		| Bit 7: Data Valid/Invalid		|
   152	 * |		|		|	 (Valid - 1, Invalid - 0)
   153	 * |		|		| Bit 8-15: Thermistor 0 - 7 presence	|
   154	 * |		|		|	    (0 present, 1 absent)	|
   155	 * ----------------------------------------------------------------------
   156	 *
   157	 */
   158	static int qcom_ec_thermal_capabilities(struct device *dev)
   159	{
   160		struct i2c_client *client = to_i2c_client(dev);
   161		struct qcom_ec *ec = i2c_get_clientdata(client);
   162		struct qcom_ec_thermal_cap *cap = &ec->thermal_cap;
   163		u8 resp[EC_THERMAL_CAP_RESP_LEN];
   164		int ret;
   165	
   166		ret = qcom_ec_read(ec, EC_THERMAL_CAP_CMD, EC_THERMAL_CAP_RESP_LEN, resp);
   167		if (ret < 0)
   168			return ret;
   169	
 > 170		cap->fan_cnt = max(EC_MAX_FAN_CNT, EC_THERMAL_FAN_CNT(resp[1]));
   171		cap->fan_type = EC_THERMAL_FAN_TYPE(resp[1]);
   172		cap->thermistor_mask = EC_THERMAL_THERMISTOR_MASK(resp[2]);
   173	
   174		dev_dbg(dev, "Fan count: %d Fan Type: %d Thermistor Mask: %d\n",
   175			cap->fan_cnt, cap->fan_type, cap->thermistor_mask);
   176	
   177		return 0;
   178	}
   179	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
> 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>
> ---
>  MAINTAINERS                            |   7 +
>  drivers/platform/arm64/Kconfig         |  12 +
>  drivers/platform/arm64/Makefile        |   1 +
>  drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
>  4 files changed, 482 insertions(+)
>  create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c
> 
> [...]
> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
> new file mode 100644
> index 000000000000..83aa869fad8f
> --- /dev/null
> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
> @@ -0,0 +1,462 @@
> [...]
> +static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
> +{
> +	int ret;
> +
> +	mutex_lock(&ec->lock);
> +	ret = i2c_smbus_read_i2c_block_data(ec->client, cmd, resp_len, resp);
> +	mutex_unlock(&ec->lock);

This mutex looks redundant to me for the current implementation. You
don't have any read-modify-write sequences and I think the I2C core
already has internal locking for the bus itself.

> [...]
> +/*
> + * 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_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);

I think it's nice to provide users a way to override the fan speed, but
is this really the main interface of the EC that we want to use for
influencing the fan speed?

As the name of the command suggests, this is a debug command that
essentially overrides the internal fan control algorithm of the EC. If
you use this to turn the fan off and then Linux hangs, I would expect
that the fan stays off until the device will eventually overheat.

I think it would be more reliable if:

 (1) The default mode of operation does not make use of the "debug mode"
     command and instead sends the internal SoC temperatures to the EC
     to help optimize the fan control. (This is what Windows does on
     Hamoa, not sure if this is still needed on Glymur?)

 (2) If we provide a way to enable the fan control debug mode, there
     should be also a way to disable it again at runtime (with
     EC_FAN_DEBUG_MODE_OFF).

Thanks,
Stephan

On 3/9/2026 2:33 PM, Stephan Gerhold wrote:
> On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
>> 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>
>> ---
>>   MAINTAINERS                            |   7 +
>>   drivers/platform/arm64/Kconfig         |  12 +
>>   drivers/platform/arm64/Makefile        |   1 +
>>   drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
>>   4 files changed, 482 insertions(+)
>>   create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c
>>
>> [...]
>> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
>> new file mode 100644
>> index 000000000000..83aa869fad8f
>> --- /dev/null
>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>> @@ -0,0 +1,462 @@
>> [...]
>> +static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
>> +{
>> +	int ret;
>> +
>> +	mutex_lock(&ec->lock);
>> +	ret = i2c_smbus_read_i2c_block_data(ec->client, cmd, resp_len, resp);
>> +	mutex_unlock(&ec->lock);
> This mutex looks redundant to me for the current implementation. You
> don't have any read-modify-write sequences and I think the I2C core
> already has internal locking for the bus itself.

Hey Stephan,
Thanks for taking time to review the series :)

Will remove this in the next re-spin.

>
>> [...]
>> +/*
>> + * 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_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);
> I think it's nice to provide users a way to override the fan speed, but
> is this really the main interface of the EC that we want to use for
> influencing the fan speed?
>
> As the name of the command suggests, this is a debug command that
> essentially overrides the internal fan control algorithm of the EC. If
> you use this to turn the fan off and then Linux hangs, I would expect
> that the fan stays off until the device will eventually overheat.
>
> I think it would be more reliable if:
>
>   (1) The default mode of operation does not make use of the "debug mode"
>       command and instead sends the internal SoC temperatures to the EC
>       to help optimize the fan control. (This is what Windows does on
>       Hamoa, not sure if this is still needed on Glymur?)

That's true, Glymur already has a way to access average SoC
temperature and even on Hamoa it can still be functional without
SoC temperature i.e. with thermistors it has access to.

The aim of the series is to expose fans as a cooling device so
that linux has a way of fan control independent to the algorithm
running on the EC.

The EC look table based fan control is still the default mode of
operation (until userspace tries to set the state of the exposed
cooling device). We have a bunch of in-flight patches which will
provide a way to tweak the pre-programmed look up tables for
the ec and send avg SoC temperature. This way we get the best
of both worlds eventually.


>
>   (2) If we provide a way to enable the fan control debug mode, there
>       should be also a way to disable it again at runtime (with
>       EC_FAN_DEBUG_MODE_OFF).

As described in the payload, having bit 3 set to 0 at offset 0x3
should turn off debug mode and EC would be back to operating
with the pre-programmed LUTs. Like you said it makes a lot of
sense to disable debug mode on ec module removal.

>
> Thanks,
> Stephan

On 3/9/26 11:04 AM, Sibi Sankar wrote:
> 
> On 3/9/2026 2:33 PM, Stephan Gerhold wrote:
>> On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
>>> 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>
>>> ---
>>>   MAINTAINERS                            |   7 +
>>>   drivers/platform/arm64/Kconfig         |  12 +
>>>   drivers/platform/arm64/Makefile        |   1 +
>>>   drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
>>>   4 files changed, 482 insertions(+)
>>>   create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c
>>>
>>> [...]
>>> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
>>> new file mode 100644
>>> index 000000000000..83aa869fad8f
>>> --- /dev/null
>>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>>> @@ -0,0 +1,462 @@
>>> [...]
>>> +static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
>>> +{
>>> +    int ret;
>>> +
>>> +    mutex_lock(&ec->lock);
>>> +    ret = i2c_smbus_read_i2c_block_data(ec->client, cmd, resp_len, resp);
>>> +    mutex_unlock(&ec->lock);
>> This mutex looks redundant to me for the current implementation. You
>> don't have any read-modify-write sequences and I think the I2C core
>> already has internal locking for the bus itself.
> 
> Hey Stephan,
> Thanks for taking time to review the series :)
> 
> Will remove this in the next re-spin.
> 
>>
>>> [...]
>>> +/*
>>> + * 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_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);
>> I think it's nice to provide users a way to override the fan speed, but
>> is this really the main interface of the EC that we want to use for
>> influencing the fan speed?
>>
>> As the name of the command suggests, this is a debug command that
>> essentially overrides the internal fan control algorithm of the EC. If
>> you use this to turn the fan off and then Linux hangs, I would expect
>> that the fan stays off until the device will eventually overheat.
>>
>> I think it would be more reliable if:
>>
>>   (1) The default mode of operation does not make use of the "debug mode"
>>       command and instead sends the internal SoC temperatures to the EC
>>       to help optimize the fan control. (This is what Windows does on
>>       Hamoa, not sure if this is still needed on Glymur?)
> 
> That's true, Glymur already has a way to access average SoC
> temperature and even on Hamoa it can still be functional without
> SoC temperature i.e. with thermistors it has access to.
> 
> The aim of the series is to expose fans as a cooling device so
> that linux has a way of fan control independent to the algorithm
> running on the EC.

I suppose the main question here is "what happens if i set the fan to zero
and put the laptop in my backpack"

The driver for M-series Macs for example, 785205fd8139 ("hwmon: Add Apple
Silicon SMC hwmon driver") hides that behind a cmdline param, since they
have no certainty. I would *assume* that if the CPU hits thermal junction
temperatures, our boards will reset, but we should be able to get a definitive
answer here.

Konrad

On Mon, Mar 09, 2026 at 12:47:33PM +0100, Konrad Dybcio wrote:
> On 3/9/26 11:04 AM, Sibi Sankar wrote:
> > On 3/9/2026 2:33 PM, Stephan Gerhold wrote:
> >> On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
> >>> 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>
> >>> ---
> >>>   MAINTAINERS                            |   7 +
> >>>   drivers/platform/arm64/Kconfig         |  12 +
> >>>   drivers/platform/arm64/Makefile        |   1 +
> >>>   drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
> >>>   4 files changed, 482 insertions(+)
> >>>   create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c
> >>>
> >>> [...]
> >>> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
> >>> new file mode 100644
> >>> index 000000000000..83aa869fad8f
> >>> --- /dev/null
> >>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
> >>> @@ -0,0 +1,462 @@
> >>> [...]
> >>> +/*
> >>> + * 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_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);
> >> I think it's nice to provide users a way to override the fan speed, but
> >> is this really the main interface of the EC that we want to use for
> >> influencing the fan speed?
> >>
> >> As the name of the command suggests, this is a debug command that
> >> essentially overrides the internal fan control algorithm of the EC. If
> >> you use this to turn the fan off and then Linux hangs, I would expect
> >> that the fan stays off until the device will eventually overheat.
> >>
> >> I think it would be more reliable if:
> >>
> >>   (1) The default mode of operation does not make use of the "debug mode"
> >>       command and instead sends the internal SoC temperatures to the EC
> >>       to help optimize the fan control. (This is what Windows does on
> >>       Hamoa, not sure if this is still needed on Glymur?)
> > 
> > That's true, Glymur already has a way to access average SoC
> > temperature and even on Hamoa it can still be functional without
> > SoC temperature i.e. with thermistors it has access to.
> > 
> > The aim of the series is to expose fans as a cooling device so
> > that linux has a way of fan control independent to the algorithm
> > running on the EC.
> 
> I suppose the main question here is "what happens if i set the fan to zero
> and put the laptop in my backpack"
> 
> The driver for M-series Macs for example, 785205fd8139 ("hwmon: Add Apple
> Silicon SMC hwmon driver") hides that behind a cmdline param, since they
> have no certainty. I would *assume* that if the CPU hits thermal junction
> temperatures, our boards will reset, but we should be able to get a definitive
> answer here.
> 

The CPUs should automatically throttle when reaching high temperatures
and Linux should also do this for the GPU. So the chance of reaching a
overtemperature state should be low as long as Linux correctly
functions. The biggest risk would be probably if Linux hangs, the
watchdog doesn't trigger and the machine is stuck in some state.

As for the hardware shutdown temperature, see commit 03f2b8eed73
("arm64: dts: qcom: x1e80100: Apply consistent critical thermal
shutdown"):

 "The firmware configures the TSENS controller with a maximum
  temperature of 120°C. When reaching that temperature, the hardware
  automatically triggers a reset of the entire platform."

The question is if you really want your device to hit 120°C. :-)

Thanks,
Stephan

On 3/9/26 12:55 PM, Stephan Gerhold wrote:
> On Mon, Mar 09, 2026 at 12:47:33PM +0100, Konrad Dybcio wrote:
>> On 3/9/26 11:04 AM, Sibi Sankar wrote:
>>> On 3/9/2026 2:33 PM, Stephan Gerhold wrote:
>>>> On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
>>>>> 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>
>>>>> ---
>>>>>   MAINTAINERS                            |   7 +
>>>>>   drivers/platform/arm64/Kconfig         |  12 +
>>>>>   drivers/platform/arm64/Makefile        |   1 +
>>>>>   drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
>>>>>   4 files changed, 482 insertions(+)
>>>>>   create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c
>>>>>
>>>>> [...]
>>>>> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
>>>>> new file mode 100644
>>>>> index 000000000000..83aa869fad8f
>>>>> --- /dev/null
>>>>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>>>>> @@ -0,0 +1,462 @@
>>>>> [...]
>>>>> +/*
>>>>> + * 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_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);
>>>> I think it's nice to provide users a way to override the fan speed, but
>>>> is this really the main interface of the EC that we want to use for
>>>> influencing the fan speed?
>>>>
>>>> As the name of the command suggests, this is a debug command that
>>>> essentially overrides the internal fan control algorithm of the EC. If
>>>> you use this to turn the fan off and then Linux hangs, I would expect
>>>> that the fan stays off until the device will eventually overheat.
>>>>
>>>> I think it would be more reliable if:
>>>>
>>>>   (1) The default mode of operation does not make use of the "debug mode"
>>>>       command and instead sends the internal SoC temperatures to the EC
>>>>       to help optimize the fan control. (This is what Windows does on
>>>>       Hamoa, not sure if this is still needed on Glymur?)
>>>
>>> That's true, Glymur already has a way to access average SoC
>>> temperature and even on Hamoa it can still be functional without
>>> SoC temperature i.e. with thermistors it has access to.
>>>
>>> The aim of the series is to expose fans as a cooling device so
>>> that linux has a way of fan control independent to the algorithm
>>> running on the EC.
>>
>> I suppose the main question here is "what happens if i set the fan to zero
>> and put the laptop in my backpack"
>>
>> The driver for M-series Macs for example, 785205fd8139 ("hwmon: Add Apple
>> Silicon SMC hwmon driver") hides that behind a cmdline param, since they
>> have no certainty. I would *assume* that if the CPU hits thermal junction
>> temperatures, our boards will reset, but we should be able to get a definitive
>> answer here.
>>
> 
> The CPUs should automatically throttle when reaching high temperatures
> and Linux should also do this for the GPU. So the chance of reaching a
> overtemperature state should be low as long as Linux correctly
> functions. The biggest risk would be probably if Linux hangs, the
> watchdog doesn't trigger and the machine is stuck in some state.
> 
> As for the hardware shutdown temperature, see commit 03f2b8eed73
> ("arm64: dts: qcom: x1e80100: Apply consistent critical thermal
> shutdown"):
> 
>  "The firmware configures the TSENS controller with a maximum
>   temperature of 120°C. When reaching that temperature, the hardware
>   automatically triggers a reset of the entire platform."
> 
> The question is if you really want your device to hit 120°C. :-)

And whether the firmware running on *your* laptop actually configures
these limits.. I would imagine that to be the case for Windows products
where the TZ comes straight from qcom, but I think someone in some thread
mentioned LMH is not properly configured on Chrome/TFA.

In any case, let's see if we can establish what/whether the EC does in
that case

Konrad

On 3/9/2026 5:40 PM, Konrad Dybcio wrote:
> On 3/9/26 12:55 PM, Stephan Gerhold wrote:
>> On Mon, Mar 09, 2026 at 12:47:33PM +0100, Konrad Dybcio wrote:
>>> On 3/9/26 11:04 AM, Sibi Sankar wrote:
>>>> On 3/9/2026 2:33 PM, Stephan Gerhold wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
>>>>>> 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>
>>>>>> ---
>>>>>>    MAINTAINERS                            |   7 +
>>>>>>    drivers/platform/arm64/Kconfig         |  12 +
>>>>>>    drivers/platform/arm64/Makefile        |   1 +
>>>>>>    drivers/platform/arm64/qcom-hamoa-ec.c | 462 +++++++++++++++++++++++++
>>>>>>    4 files changed, 482 insertions(+)
>>>>>>    create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c
>>>>>>
>>>>>> [...]
>>>>>> diff --git a/drivers/platform/arm64/qcom-hamoa-ec.c b/drivers/platform/arm64/qcom-hamoa-ec.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..83aa869fad8f
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>>>>>> @@ -0,0 +1,462 @@
>>>>>> [...]
>>>>>> +/*
>>>>>> + * 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_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);
>>>>> I think it's nice to provide users a way to override the fan speed, but
>>>>> is this really the main interface of the EC that we want to use for
>>>>> influencing the fan speed?
>>>>>
>>>>> As the name of the command suggests, this is a debug command that
>>>>> essentially overrides the internal fan control algorithm of the EC. If
>>>>> you use this to turn the fan off and then Linux hangs, I would expect
>>>>> that the fan stays off until the device will eventually overheat.
>>>>>
>>>>> I think it would be more reliable if:
>>>>>
>>>>>    (1) The default mode of operation does not make use of the "debug mode"
>>>>>        command and instead sends the internal SoC temperatures to the EC
>>>>>        to help optimize the fan control. (This is what Windows does on
>>>>>        Hamoa, not sure if this is still needed on Glymur?)
>>>> That's true, Glymur already has a way to access average SoC
>>>> temperature and even on Hamoa it can still be functional without
>>>> SoC temperature i.e. with thermistors it has access to.
>>>>
>>>> The aim of the series is to expose fans as a cooling device so
>>>> that linux has a way of fan control independent to the algorithm
>>>> running on the EC.
>>> I suppose the main question here is "what happens if i set the fan to zero
>>> and put the laptop in my backpack"
>>>
>>> The driver for M-series Macs for example, 785205fd8139 ("hwmon: Add Apple
>>> Silicon SMC hwmon driver") hides that behind a cmdline param, since they
>>> have no certainty. I would *assume* that if the CPU hits thermal junction
>>> temperatures, our boards will reset, but we should be able to get a definitive
>>> answer here.
>>>
>> The CPUs should automatically throttle when reaching high temperatures
>> and Linux should also do this for the GPU. So the chance of reaching a
>> overtemperature state should be low as long as Linux correctly
>> functions. The biggest risk would be probably if Linux hangs, the
>> watchdog doesn't trigger and the machine is stuck in some state.
>>
>> As for the hardware shutdown temperature, see commit 03f2b8eed73
>> ("arm64: dts: qcom: x1e80100: Apply consistent critical thermal
>> shutdown"):
>>
>>   "The firmware configures the TSENS controller with a maximum
>>    temperature of 120°C. When reaching that temperature, the hardware
>>    automatically triggers a reset of the entire platform."
>>
>> The question is if you really want your device to hit 120°C. :-)
> And whether the firmware running on *your* laptop actually configures
> these limits.. I would imagine that to be the case for Windows products
> where the TZ comes straight from qcom, but I think someone in some thread
> mentioned LMH is not properly configured on Chrome/TFA.
>
> In any case, let's see if we can establish what/whether the EC does in
> that case


The "debug mode" which is used to control fan is paused during the
following conditions:

1) When the EC receives EC_MODERN_STANDY_CMD enter cmd
2) When SoC enters deep sleep fan pwm is turned off
3) When external processors like soccp put fan constraints on the EC

So when you do set the fan to 0 and put it in your backpack :P
It would enter suspend, the EC would take back fan control.
i.e. the LUT based flow that it follows by default. That would
imply it would be the same behavior as seen on Windows?

Either way I still think we all are in agreement here. This series just
exposes fan control knobs to the kernel and doesn't set any fan
speed by default. The LUT based EC fan control would still be the
default, the patches to improve this are in-flight i.e. ways to send
avg SoC temp, update LUT and select available profiles. With all
of these in place EC would be in good shape in the Qualcomm
reference devices.

>
> Konrad

On 3/10/26 5:58 AM, Sibi Sankar wrote:
> 
> On 3/9/2026 5:40 PM, Konrad Dybcio wrote:
>> On 3/9/26 12:55 PM, Stephan Gerhold wrote:
>>> On Mon, Mar 09, 2026 at 12:47:33PM +0100, Konrad Dybcio wrote:
>>>> On 3/9/26 11:04 AM, Sibi Sankar wrote:
>>>>> On 3/9/2026 2:33 PM, Stephan Gerhold wrote:
>>>>>> On Mon, Mar 09, 2026 at 05:06:43AM +0530, Sibi Sankar wrote:
>>>>>>> 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.

[...]

>>> The question is if you really want your device to hit 120°C. :-)
>> And whether the firmware running on *your* laptop actually configures
>> these limits.. I would imagine that to be the case for Windows products
>> where the TZ comes straight from qcom, but I think someone in some thread
>> mentioned LMH is not properly configured on Chrome/TFA.
>>
>> In any case, let's see if we can establish what/whether the EC does in
>> that case
> 
> 
> The "debug mode" which is used to control fan is paused during the
> following conditions:
> 
> 1) When the EC receives EC_MODERN_STANDY_CMD enter cmd
> 2) When SoC enters deep sleep fan pwm is turned off
> 3) When external processors like soccp put fan constraints on the EC
> 
> So when you do set the fan to 0 and put it in your backpack :P
> It would enter suspend, the EC would take back fan control.
> i.e. the LUT based flow that it follows by default. That would
> imply it would be the same behavior as seen on Windows?
> 
> Either way I still think we all are in agreement here. This series just
> exposes fan control knobs to the kernel and doesn't set any fan
> speed by default. The LUT based EC fan control would still be the
> default, the patches to improve this are in-flight i.e. ways to send
> avg SoC temp, update LUT and select available profiles. With all
> of these in place EC would be in good shape in the Qualcomm
> reference devices.

Right, and I suppose we're in agreement that treating the

* user sets fan to 0
* failed to suspend
* 1) didn't happen
* fan still off

chain as an oddball case is OK

Konrad