[PATCH V3 0/5] Add driver for EC found on Qualcomm reference devices

[PATCH V3 0/5] Add driver for EC found on Qualcomm reference devices

[Sibi Sankar]

From: Sibi Sankar <quic_sibis@quicinc.com>

Add Embedded controller driver support for Hamoa/Purwa/Glymur Qualcomm
reference boards, which run on IT8987 and Nuvoton MCUs respectively. It
handles fan control, temperature sensors, access to EC state changes and
supports reporting suspend entry/exit to the EC.

Changes in v3:
  - Revamp the bindings and driver to support generic ec specification
    that works across Qualcomm Hamoa/Purwa and Glymur reference devices.
  - Add ec nodes to Hamoa/Purwa CRDs and IOT-EVKs.
  - Add ec node to Glymur CRDs.
  - Link to v2: https://lore.kernel.org/lkml/20241219200821.8328-1-maccraft123mc@gmail.com/
  - Link to v1: https://lore.kernel.org/lkml/20240927185345.3680-1-maccraft123mc@gmail.com/

Maya Matuszczyk (1):
  dt-bindings: embedded-controller: Add EC bindings for Qualcomm
    reference devices

Sibi Sankar (4):
  platform: arm64: Add driver for EC found on Qualcomm reference devices
  arm64: dts: qcom: glymur-crd: Add Embedded controller node
  arm64: dts: qcom: x1-crd: Add Embedded controller node
  arm64: dts: qcom: hamoa-iot-evk: Add Embedded controller node

 .../embedded-controller/qcom,hamoa-ec.yaml    |  52 ++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/qcom/glymur-crd.dts       |  22 +
 arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts    |  10 +
 arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi   |   6 +
 arch/arm64/boot/dts/qcom/x1-crd.dtsi          |  16 +
 drivers/platform/arm64/Kconfig                |  12 +
 drivers/platform/arm64/Makefile               |   1 +
 drivers/platform/arm64/qcom-hamoa-ec.c        | 462 ++++++++++++++++++
 9 files changed, 588 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
 create mode 100644 drivers/platform/arm64/qcom-hamoa-ec.c


base-commit: a0ae2a256046c0c5d3778d1a194ff2e171f16e5f
-- 
2.34.1

[PATCH V3 1/5] dt-bindings: embedded-controller: Add EC bindings for Qualcomm reference devices

[Sibi Sankar]

From: Maya Matuszczyk <maccraft123mc@gmail.com>

Add bindings for the EC firmware running on Hamoa/Purwa and Glymur
reference devices, which run on IT8987 and Nuvoton MCUs respectively.

Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
Co-developed-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
---
 .../embedded-controller/qcom,hamoa-ec.yaml    | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml

diff --git a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
new file mode 100644
index 000000000000..ea093b71d269
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/embedded-controller/qcom,hamoa-ec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Hamoa Embedded Controller.
+
+maintainers:
+  - Sibi Sankar <sibi.sankar@oss.qualcomm.com>
+
+description:
+  Qualcomm Snapdragon based Hamoa/Purwa and Glymur reference devices have an
+  EC running on IT8987 and Nuvoton MCU chips respectively. The EC handles things
+  like fan control, temperature sensors, access to EC internal state changes.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,glymur-nuvoton-ec
+          - qcom,hamoa-it8987-ec
+      - const: qcom,hamoa-ec
+
+  reg:
+    const: 0x76
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        embedded-controller@76 {
+            compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
+            reg = <0x76>;
+
+            interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_HIGH>;
+        };
+    };
+...
-- 
2.34.1

Re: [PATCH V3 1/5] dt-bindings: embedded-controller: Add EC bindings for Qualcomm reference devices

[Krzysztof Kozlowski]

On Mon, Mar 09, 2026 at 05:06:42AM +0530, Sibi Sankar wrote:
> From: Maya Matuszczyk <maccraft123mc@gmail.com>
> 
> Add bindings for the EC firmware running on Hamoa/Purwa and Glymur
> reference devices, which run on IT8987 and Nuvoton MCUs respectively.
> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Co-developed-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> ---
>  .../embedded-controller/qcom,hamoa-ec.yaml    | 52 +++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
> 
> diff --git a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
> new file mode 100644
> index 000000000000..ea093b71d269
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/embedded-controller/qcom,hamoa-ec.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Hamoa Embedded Controller.

Please implement the feedback from v2.

> +
> +maintainers:
> +  - Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> +
> +description:
> +  Qualcomm Snapdragon based Hamoa/Purwa and Glymur reference devices have an
> +  EC running on IT8987 and Nuvoton MCU chips respectively. The EC handles things
> +  like fan control, temperature sensors, access to EC internal state changes.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,glymur-nuvoton-ec

nuvoton is name of the company, so it's too generic to describe a
component. It's like calling it "qcom,glymur-qcom-ec". How many EC do
you have there?

> +          - qcom,hamoa-it8987-ec

I don't understand this compatible. You already have hamoa.

> +      - const: qcom,hamoa-ec

So which EC is this?

Best regards,
Krzysztof

Re: [PATCH V3 1/5] dt-bindings: embedded-controller: Add EC bindings for Qualcomm reference devices

[Sibi Sankar]

On 3/9/2026 12:53 PM, Krzysztof Kozlowski wrote:
> On Mon, Mar 09, 2026 at 05:06:42AM +0530, Sibi Sankar wrote:
>> From: Maya Matuszczyk <maccraft123mc@gmail.com>
>>
>> Add bindings for the EC firmware running on Hamoa/Purwa and Glymur
>> reference devices, which run on IT8987 and Nuvoton MCUs respectively.
>>
>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>> Co-developed-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> ---
>>   .../embedded-controller/qcom,hamoa-ec.yaml    | 52 +++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
>> new file mode 100644
>> index 000000000000..ea093b71d269
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
>> @@ -0,0 +1,52 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/embedded-controller/qcom,hamoa-ec.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Hamoa Embedded Controller.
> Please implement the feedback from v2.

Ack, sorry missed it.

>
>> +
>> +maintainers:
>> +  - Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> +
>> +description:
>> +  Qualcomm Snapdragon based Hamoa/Purwa and Glymur reference devices have an
>> +  EC running on IT8987 and Nuvoton MCU chips respectively. The EC handles things
>> +  like fan control, temperature sensors, access to EC internal state changes.
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - qcom,glymur-nuvoton-ec
> nuvoton is name of the company, so it's too generic to describe a
> component. It's like calling it "qcom,glymur-qcom-ec". How many EC do
> you have there?

Ack, will get this updated.

>> +          - qcom,hamoa-it8987-ec
> I don't understand this compatible. You already have hamoa.
>
>> +      - const: qcom,hamoa-ec
> So which EC is this?

Will update the compatibles when consensus is achieved
on patch 4 thread.

>
> Best regards,
> Krzysztof
>

Re: [PATCH V3 1/5] dt-bindings: embedded-controller: Add EC bindings for Qualcomm reference devices

[Dmitry Baryshkov]

On Mon, Mar 09, 2026 at 05:06:42AM +0530, Sibi Sankar wrote:
> From: Maya Matuszczyk <maccraft123mc@gmail.com>
> 
> Add bindings for the EC firmware running on Hamoa/Purwa and Glymur
> reference devices, which run on IT8987 and Nuvoton MCUs respectively.
> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> Co-developed-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> ---
>  .../embedded-controller/qcom,hamoa-ec.yaml    | 52 +++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml

Looking at the DSDT database at [1], several laptops are using the same
protocol for the EC. Do we plan to use this driver for other
Linux-supported laptops too?

> 

-- 
With best wishes
Dmitry

Re: [PATCH V3 1/5] dt-bindings: embedded-controller: Add EC bindings for Qualcomm reference devices

[Sibi Sankar]

On 3/10/2026 2:36 AM, Dmitry Baryshkov wrote:
> On Mon, Mar 09, 2026 at 05:06:42AM +0530, Sibi Sankar wrote:
>> From: Maya Matuszczyk <maccraft123mc@gmail.com>
>>
>> Add bindings for the EC firmware running on Hamoa/Purwa and Glymur
>> reference devices, which run on IT8987 and Nuvoton MCUs respectively.
>>
>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
>> Co-developed-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> ---
>>   .../embedded-controller/qcom,hamoa-ec.yaml    | 52 +++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
> Looking at the DSDT database at [1], several laptops are using the same
> protocol for the EC. Do we plan to use this driver for other
> Linux-supported laptops too?


It's very likely that the fan control/thermal bits are identical so
re-using this driver would make sense. That said we have no insight
into how the EC firmware is extended on the other laptops, so
adding support to them would still involve a bit of experimentation
and access to those devices.

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

[Sibi Sankar]

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

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

[Stephan Gerhold]

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

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

[Sibi Sankar]

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

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

[Konrad Dybcio]

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

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

[Stephan Gerhold]

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

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

[Konrad Dybcio]

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

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

[Sibi Sankar]

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

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

[Konrad Dybcio]

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

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

[kernel test robot]

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

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

[kernel test robot]

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

[PATCH V3 3/5] arm64: dts: qcom: glymur-crd: Add Embedded controller node

[Sibi Sankar]

Add embedded controller node for Glymur CRDs which adds fan control,
temperature sensors, access to EC state changes through SCI events
and suspend entry/exit notifications to the EC.

Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/glymur-crd.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dts b/arch/arm64/boot/dts/qcom/glymur-crd.dts
index 877945319012..94abef7f0f1f 100644
--- a/arch/arm64/boot/dts/qcom/glymur-crd.dts
+++ b/arch/arm64/boot/dts/qcom/glymur-crd.dts
@@ -367,6 +367,22 @@ vreg_l4h_e0_1p2: ldo4 {
 	};
 };
 
+&i2c9 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+
+	embedded-controller@76 {
+		compatible = "qcom,glymur-nuvoton-ec", "qcom,hamoa-ec";
+		reg = <0x76>;
+
+		interrupts-extended = <&tlmm 66 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&ec_int_n_default>;
+		pinctrl-names = "default";
+	};
+};
+
 &pcie3b {
 	vddpe-3v3-supply = <&vreg_nvmesec>;
 
@@ -490,6 +506,12 @@ &tlmm {
 			       <10 2>, /* OOB UART */
 			       <44 4>; /* Security SPI (TPM) */
 
+	ec_int_n_default: ec-int-n-state {
+		pins = "gpio66";
+		function = "gpio";
+		bias-disable;
+	};
+
 	pcie4_default: pcie4-default-state {
 		clkreq-n-pins {
 			pins = "gpio147";
-- 
2.34.1

[PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Sibi Sankar]

Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
temperature sensors, access to EC internal state changes and suspend
entry/exit notifications to the EC.

Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
index ded96fb43489..29a1aeb98353 100644
--- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
@@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
 
 		#phy-cells = <0>;
 	};
+
+	embedded-controller@76 {
+		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
+		reg = <0x76>;
+
+		interrupts-extended = <&tlmm 66 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&ec_int_n_default>;
+		pinctrl-names = "default";
+	};
 };
 
 &i2c7 {
@@ -1485,6 +1495,12 @@ &tlmm {
 			       <44 4>, /* SPI (TPM) */
 			       <238 1>; /* UFS Reset */
 
+	ec_int_n_default: ec-int-n-state {
+		pins = "gpio66";
+		function = "gpio";
+		bias-disable;
+	};
+
 	edp_reg_en: edp-reg-en-state {
 		pins = "gpio70";
 		function = "gpio";
-- 
2.34.1

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Krzysztof Kozlowski]

On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
> temperature sensors, access to EC internal state changes and suspend
> entry/exit notifications to the EC.
> 
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> index ded96fb43489..29a1aeb98353 100644
> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>  
>  		#phy-cells = <0>;
>  	};
> +
> +	embedded-controller@76 {
> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";

I don't see updates to other x1e boards, thus my arguments from v2 stay
valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
it. All of other Hamoa boards apparently do not have it.

Best regards,
Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Sibi Sankar]

On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>> temperature sensors, access to EC internal state changes and suspend
>> entry/exit notifications to the EC.
>>
>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>> index ded96fb43489..29a1aeb98353 100644
>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>   
>>   		#phy-cells = <0>;
>>   	};
>> +
>> +	embedded-controller@76 {
>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
> I don't see updates to other x1e boards, thus my arguments from v2 stay
> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
> it. All of other Hamoa boards apparently do not have it.


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

What other Hamoa boards are you referring to? The series
mentions that the driver and bindings is meant for Qualcomm
Hamoa/Purwa/Glymur "reference" devices, so it only covers
CRD and IOT-EVK. It definitely does not cover all Hamoa boards
boards like you are assuming.

> Best regards,
> Krzysztof
>

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Krzysztof Kozlowski]

On 09/03/2026 10:03, Sibi Sankar wrote:
> 
> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>> temperature sensors, access to EC internal state changes and suspend
>>> entry/exit notifications to the EC.
>>>
>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>> ---
>>>   arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>> index ded96fb43489..29a1aeb98353 100644
>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>   
>>>   		#phy-cells = <0>;
>>>   	};
>>> +
>>> +	embedded-controller@76 {
>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>> it. All of other Hamoa boards apparently do not have it.
> 
> 
> Hey Krzysztof,
> Thanks for taking time to review the series :)
> 
> What other Hamoa boards are you referring to? The series
> mentions that the driver and bindings is meant for Qualcomm
> Hamoa/Purwa/Glymur "reference" devices, so it only covers
> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
> boards like you are assuming.

hamoa-ec compatible implies that and that's something I raised in v2
already. You need a specific compatible.


Best regards,
Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Sibi Sankar]

On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
> On 09/03/2026 10:03, Sibi Sankar wrote:
>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>> temperature sensors, access to EC internal state changes and suspend
>>>> entry/exit notifications to the EC.
>>>>
>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>> index ded96fb43489..29a1aeb98353 100644
>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>    
>>>>    		#phy-cells = <0>;
>>>>    	};
>>>> +
>>>> +	embedded-controller@76 {
>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>> it. All of other Hamoa boards apparently do not have it.
>>
>> Hey Krzysztof,
>> Thanks for taking time to review the series :)
>>
>> What other Hamoa boards are you referring to? The series
>> mentions that the driver and bindings is meant for Qualcomm
>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>> boards like you are assuming.
> hamoa-ec compatible implies that and that's something I raised in v2
> already. You need a specific compatible.


Hamoa/Glymur reference devices can have different EC MCUs
depending on the SKU. This introduces the need to deal with
possibility of quirks and bugs introduced by these differences.
Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
reference devices run on NPCX498/488. This pretty much was
the rationale to make the MCU part of the compatible. Anyway
I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
and add specific compatibles when we upstream those boards?

>
>
> Best regards,
> Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Krzysztof Kozlowski]

On 09/03/2026 11:51, Sibi Sankar wrote:
> 
> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>> entry/exit notifications to the EC.
>>>>>
>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>> ---
>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>    1 file changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>    
>>>>>    		#phy-cells = <0>;
>>>>>    	};
>>>>> +
>>>>> +	embedded-controller@76 {
>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>> it. All of other Hamoa boards apparently do not have it.
>>>
>>> Hey Krzysztof,
>>> Thanks for taking time to review the series :)
>>>
>>> What other Hamoa boards are you referring to? The series
>>> mentions that the driver and bindings is meant for Qualcomm
>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>> boards like you are assuming.
>> hamoa-ec compatible implies that and that's something I raised in v2
>> already. You need a specific compatible.
> 
> 
> Hamoa/Glymur reference devices can have different EC MCUs
> depending on the SKU. This introduces the need to deal with
> possibility of quirks and bugs introduced by these differences.
> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
> reference devices run on NPCX498/488. This pretty much was

None of these answer my comments from here and v2.

> the rationale to make the MCU part of the compatible. Anyway
> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now

No. You cannot add a generic compatible when you claim it is not even
correct - "different EC depending on the SKU".

Best regards,
Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Konrad Dybcio]

On 3/9/26 11:53 AM, Krzysztof Kozlowski wrote:
> On 09/03/2026 11:51, Sibi Sankar wrote:
>>
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>    1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>    
>>>>>>    		#phy-cells = <0>;
>>>>>>    	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>>
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
> 
> None of these answer my comments from here and v2.
> 
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
> 
> No. You cannot add a generic compatible when you claim it is not even
> correct - "different EC depending on the SKU".

I agree, this name isn't really the best. We don't really have a better
"official one" though. Perhaps something like "qcom,compute-ec"?

"qcom,reference-compute-ec-that-happens-to-be-found-on-boards-featuring-
hamoa-glymur-and-derivative-socs-running-windows-by-design"?

Konrad

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Krzysztof Kozlowski]

On 09/03/2026 12:48, Konrad Dybcio wrote:
> On 3/9/26 11:53 AM, Krzysztof Kozlowski wrote:
>> On 09/03/2026 11:51, Sibi Sankar wrote:
>>>
>>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>>> entry/exit notifications to the EC.
>>>>>>>
>>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>>> ---
>>>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>>    1 file changed, 16 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>>    
>>>>>>>    		#phy-cells = <0>;
>>>>>>>    	};
>>>>>>> +
>>>>>>> +	embedded-controller@76 {
>>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>>> it. All of other Hamoa boards apparently do not have it.
>>>>>
>>>>> Hey Krzysztof,
>>>>> Thanks for taking time to review the series :)
>>>>>
>>>>> What other Hamoa boards are you referring to? The series
>>>>> mentions that the driver and bindings is meant for Qualcomm
>>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>>> boards like you are assuming.
>>>> hamoa-ec compatible implies that and that's something I raised in v2
>>>> already. You need a specific compatible.
>>>
>>>
>>> Hamoa/Glymur reference devices can have different EC MCUs
>>> depending on the SKU. This introduces the need to deal with
>>> possibility of quirks and bugs introduced by these differences.
>>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>>> reference devices run on NPCX498/488. This pretty much was
>>
>> None of these answer my comments from here and v2.
>>
>>> the rationale to make the MCU part of the compatible. Anyway
>>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
>>
>> No. You cannot add a generic compatible when you claim it is not even
>> correct - "different EC depending on the SKU".
> 
> I agree, this name isn't really the best. We don't really have a better
> "official one" though. Perhaps something like "qcom,compute-ec"?
> 
> "qcom,reference-compute-ec-that-happens-to-be-found-on-boards-featuring-
> hamoa-glymur-and-derivative-socs-running-windows-by-design"?

I asked at v2 and here I asked twice, so three times already. You need
compatible for board. Nothing else.


Best regards,
Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Sibi Sankar]

On 3/9/2026 4:23 PM, Krzysztof Kozlowski wrote:
> On 09/03/2026 11:51, Sibi Sankar wrote:
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>     1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>     
>>>>>>     		#phy-cells = <0>;
>>>>>>     	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
> None of these answer my comments from here and v2.
>
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
> No. You cannot add a generic compatible when you claim it is not even
> correct - "different EC depending on the SKU".

https://lore.kernel.org/lkml/1336e1c3-6ee9-4a19-976b-0bfdc02fc8e6@quicinc.com/

The explanation mentioned ^^ was the motivation
to have a extremely generic compatible as a fallback
since the firmware basically implements the same
exact specification.

https://lore.kernel.org/lkml/def949f2-301d-4edc-b303-0fbe02a18c3c@kernel.org/

I'll stick to your suggestion mentioned ^^ and use the
following compatibles instead:

qcom,hamoa-crd-it8987-ec
qcom,hamoa-iot-evk-it8987-ec
qcom,glymur-crd-ncpx498s-ec

I'll continue to use the hamoa-crd based compatible
as the fallback since they implement the same firmware
on different MCUs.

+properties: + compatible: + items: + - enum: + - 
qcom,glymur-crd-npcx498s-ec + - qcom,hamoa-iot-evk-it8987-ec + - const: 
qcom,hamoa-crd-it8987-ec Thanks again!

>
> Best regards,
> Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Dmitry Baryshkov]

On Mon, Mar 09, 2026 at 04:21:50PM +0530, Sibi Sankar wrote:
> 
> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
> > On 09/03/2026 10:03, Sibi Sankar wrote:
> > > On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
> > > > On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
> > > > > Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
> > > > > temperature sensors, access to EC internal state changes and suspend
> > > > > entry/exit notifications to the EC.
> > > > > 
> > > > > Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> > > > > ---
> > > > >    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
> > > > >    1 file changed, 16 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> > > > > index ded96fb43489..29a1aeb98353 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> > > > > @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
> > > > >    		#phy-cells = <0>;
> > > > >    	};
> > > > > +
> > > > > +	embedded-controller@76 {
> > > > > +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
> > > > I don't see updates to other x1e boards, thus my arguments from v2 stay
> > > > valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
> > > > it. All of other Hamoa boards apparently do not have it.
> > > 
> > > Hey Krzysztof,
> > > Thanks for taking time to review the series :)
> > > 
> > > What other Hamoa boards are you referring to? The series
> > > mentions that the driver and bindings is meant for Qualcomm
> > > Hamoa/Purwa/Glymur "reference" devices, so it only covers
> > > CRD and IOT-EVK. It definitely does not cover all Hamoa boards
> > > boards like you are assuming.
> > hamoa-ec compatible implies that and that's something I raised in v2
> > already. You need a specific compatible.
> 
> 
> Hamoa/Glymur reference devices can have different EC MCUs
> depending on the SKU. This introduces the need to deal with
> possibility of quirks and bugs introduced by these differences.
> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
> reference devices run on NPCX498/488. This pretty much was
> the rationale to make the MCU part of the compatible. Anyway
> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
> and add specific compatibles when we upstream those boards?

Why do you need a generic compat at all? Glancing at the database, at
least the following laptops seem to have similar protocol (I might be
wrong, this is based on a very quick glance):

acer-sfa14-11
asus-vivobook-s-15
asus-vivobook-s15-q5507
asus-vivobook-s15-s5507
honor-magicbook-art-14
honor-mro-521
hp-elitebook-6-g1q
hp-omnibook-5-14-he0xxx
lenovo-ideacentre-01q8x10
lenovo-ideapad-slim3x-15q8x10-WCN7850
lenovo-thinkpad-t14s-120hz-64gb
lenovo-thinkpad-t14s
lenovo-yoga-slim-7x
medion-14-s1-elite-sprchrgd
medion-14-s1-sprchrgd
qualcomm-snapdragon-dev-kit
tuxedo-elite-14-gen1

I assume that some of them might be false positives and others will use
vendor (or device) extensions to your protocol.

I'd suggest implicitly using "qcom,hamoa-crd-ec" / "qcom,glymyr-crd-ec",
because then we can use something like "asus,vivobook-s15-ec" to
identify the EC on VivoBook S15.

> 
> > 
> > 
> > Best regards,
> > Krzysztof

-- 
With best wishes
Dmitry

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Sibi Sankar]

On 3/10/2026 2:43 AM, Dmitry Baryshkov wrote:
> On Mon, Mar 09, 2026 at 04:21:50PM +0530, Sibi Sankar wrote:
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>     1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>     		#phy-cells = <0>;
>>>>>>     	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
>> and add specific compatibles when we upstream those boards?
> Why do you need a generic compat at all? Glancing at the database, at
> least the following laptops seem to have similar protocol (I might be
> wrong, this is based on a very quick glance):
>
> acer-sfa14-11
> asus-vivobook-s-15
> asus-vivobook-s15-q5507
> asus-vivobook-s15-s5507
> honor-magicbook-art-14
> honor-mro-521
> hp-elitebook-6-g1q
> hp-omnibook-5-14-he0xxx
> lenovo-ideacentre-01q8x10
> lenovo-ideapad-slim3x-15q8x10-WCN7850
> lenovo-thinkpad-t14s-120hz-64gb
> lenovo-thinkpad-t14s
> lenovo-yoga-slim-7x
> medion-14-s1-elite-sprchrgd
> medion-14-s1-sprchrgd
> qualcomm-snapdragon-dev-kit
> tuxedo-elite-14-gen1
>
> I assume that some of them might be false positives and others will use
> vendor (or device) extensions to your protocol.
>
> I'd suggest implicitly using "qcom,hamoa-crd-ec" / "qcom,glymyr-crd-ec",
> because then we can use something like "asus,vivobook-s15-ec" to
> identify the EC on VivoBook S15.

Ack, this was the consensus reached as well.

>
>>>
>>> Best regards,
>>> Krzysztof

Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

[Konrad Dybcio]

On 3/9/26 10:13 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 09, 2026 at 04:21:50PM +0530, Sibi Sankar wrote:
>>
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>    1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>    		#phy-cells = <0>;
>>>>>>    	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>>
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
>> and add specific compatibles when we upstream those boards?
> 
> Why do you need a generic compat at all? Glancing at the database, at
> least the following laptops seem to have similar protocol (I might be
> wrong, this is based on a very quick glance):
> 
> acer-sfa14-11
> asus-vivobook-s-15
> asus-vivobook-s15-q5507
> asus-vivobook-s15-s5507
> honor-magicbook-art-14
> honor-mro-521
> hp-elitebook-6-g1q
> hp-omnibook-5-14-he0xxx
> lenovo-ideacentre-01q8x10
> lenovo-ideapad-slim3x-15q8x10-WCN7850
> lenovo-thinkpad-t14s-120hz-64gb
> lenovo-thinkpad-t14s
> lenovo-yoga-slim-7x
> medion-14-s1-elite-sprchrgd
> medion-14-s1-sprchrgd
> qualcomm-snapdragon-dev-kit
> tuxedo-elite-14-gen1
> 
> I assume that some of them might be false positives and others will use
> vendor (or device) extensions to your protocol.
> 
> I'd suggest implicitly using "qcom,hamoa-crd-ec" / "qcom,glymyr-crd-ec",
> because then we can use something like "asus,vivobook-s15-ec" to
> identify the EC on VivoBook S15.

+1

Konrad

[PATCH V3 5/5] arm64: dts: qcom: hamoa-iot-evk: Add Embedded controller node

[Sibi Sankar]

Add embedded controller node for Hamoa IOT EVK boards which adds fan
control, temperature sensors, access to EC internal state changes and
suspend entry/exit notifications to the EC.

Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts  | 10 ++++++++++
 arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi |  6 ++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts b/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts
index 630642baa435..3cbbc4a0dfdf 100644
--- a/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts
+++ b/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts
@@ -799,6 +799,16 @@ eusb6_repeater: redriver@4f {
 		pinctrl-0 = <&eusb6_reset_n>;
 		pinctrl-names = "default";
 	};
+
+	embedded-controller@76 {
+		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
+		reg = <0x76>;
+
+		interrupts-extended = <&tlmm 66 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&ec_int_n_default>;
+		pinctrl-names = "default";
+	};
 };
 
 &i2c7 {
diff --git a/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi b/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi
index b8e3e04a6fbd..763399393daf 100644
--- a/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi
@@ -475,6 +475,12 @@ &remoteproc_cdsp {
 &tlmm {
 	gpio-reserved-ranges = <34 2>; /* TPM LP & INT */
 
+	ec_int_n_default: ec-int-n-state {
+		pins = "gpio66";
+		function = "gpio";
+		bias-disable;
+	};
+
 	pcie3_default: pcie3-default-state {
 		clkreq-n-pins {
 			pins = "gpio144";
-- 
2.34.1