Re: [PATCH v7 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
From: Anvesh Jain P
Date: Wed Apr 08 2026 - 02:05:57 EST
On 3/30/2026 12:14 PM, Anvesh Jain P wrote:
>
>
> On 3/27/2026 5:20 PM, Ilpo Järvinen wrote:
>> On Fri, 27 Mar 2026, Anvesh Jain P wrote:
>>
>>> From: Sibi Sankar <sibi.sankar@xxxxxxxxxxxxxxxx>
>>>
>>> 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@xxxxxxxxx>
>>> Signed-off-by: Maya Matuszczyk <maccraft123mc@xxxxxxxxx>
>>> Signed-off-by: Sibi Sankar <sibi.sankar@xxxxxxxxxxxxxxxx>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
>>> Acked-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
>>> Co-developed-by: Anvesh Jain P <anvesh.p@xxxxxxxxxxxxxxxx>
>>> Signed-off-by: Anvesh Jain P <anvesh.p@xxxxxxxxxxxxxxxx>
>>> ---
>>> MAINTAINERS | 8 +
>>> drivers/platform/arm64/Kconfig | 12 +
>>> drivers/platform/arm64/Makefile | 1 +
>>> drivers/platform/arm64/qcom-hamoa-ec.c | 451 +++++++++++++++++++++++++++++++++
>>> 4 files changed, 472 insertions(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 30ca84404976..536dfd9adff4 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -21804,6 +21804,14 @@ F: Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>>> F: drivers/misc/fastrpc.c
>>> F: include/uapi/misc/fastrpc.h
>>>
>>> +QUALCOMM HAMOA EMBEDDED CONTROLLER DRIVER
>>> +M: Anvesh Jain P <anvesh.p@xxxxxxxxxxxxxxxx>
>>> +M: Sibi Sankar <sibi.sankar@xxxxxxxxxxxxxxxx>
>>> +L: linux-arm-msm@xxxxxxxxxxxxxxx
>>> +S: Maintained
>>> +F: Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-ec.yaml
>>> +F: drivers/platform/arm64/qcom-hamoa-ec.c
>>> +
>>> QUALCOMM HEXAGON ARCHITECTURE
>>> M: Brian Cain <brian.cain@xxxxxxxxxxxxxxxx>
>>> L: linux-hexagon@xxxxxxxxxxxxxxx
>>> 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..0f883130ac9a
>>> --- /dev/null
>>> +++ b/drivers/platform/arm64/qcom-hamoa-ec.c
>>> @@ -0,0 +1,451 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2024 Maya Matuszczyk <maccraft123mc@xxxxxxxxx>
>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/bits.h>
>>> +#include <linux/device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/pm.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/thermal.h>
>>> +
>>> +#define EC_SCI_EVT_READ_CMD 0x05
>>> +#define EC_FW_VERSION_CMD 0x0e
>>> +#define EC_MODERN_STANDBY_CMD 0x23
>>> +#define EC_FAN_DBG_CONTROL_CMD 0x30
>>> +#define EC_SCI_EVT_CONTROL_CMD 0x35
>>> +#define EC_THERMAL_CAP_CMD 0x42
>>> +
>>> +#define EC_FW_VERSION_RESP_LEN 4
>>> +#define EC_THERMAL_CAP_RESP_LEN 3
>>> +#define EC_FAN_DEBUG_CMD_LEN 6
>>> +#define EC_FAN_SPEED_DATA_SIZE 4
>>> +
>>> +#define EC_MODERN_STANDBY_ENTER 0x01
>>> +#define EC_MODERN_STANDBY_EXIT 0x00
>>> +
>>> +#define EC_FAN_DEBUG_MODE_OFF 0
>>> +#define EC_FAN_DEBUG_MODE_ON BIT(0)
>>
>> Add include for BIT().
>>
>
> Added <linux/bits.h> in v6 to address this.
>
>>> +#define EC_FAN_ON BIT(1)
>>> +#define EC_FAN_DEBUG_TYPE_PWM BIT(2)
>>> +#define EC_MAX_FAN_CNT 2
>>> +#define EC_FAN_NAME_SIZE 20
>>> +#define EC_FAN_MAX_PWM 255
>>> +
>>> +enum qcom_ec_sci_events {
>>> + EC_FAN1_STATUS_CHANGE_EVT = 0x30,
>>> + EC_FAN2_STATUS_CHANGE_EVT,
>>> + EC_FAN1_SPEED_CHANGE_EVT,
>>> + EC_FAN2_SPEED_CHANGE_EVT,
>>> + EC_NEW_LUT_SET_EVT,
>>> + EC_FAN_PROFILE_SWITCH_EVT,
>>> + EC_THERMISTOR_1_THRESHOLD_CROSS_EVT,
>>> + EC_THERMISTOR_2_THRESHOLD_CROSS_EVT,
>>> + EC_THERMISTOR_3_THRESHOLD_CROSS_EVT,
>>> + /* Reserved: 0x39 - 0x3c/0x3f */
>>> + EC_RECOVERED_FROM_RESET_EVT = 0x3d,
>>> +};
>>> +
>>> +struct qcom_ec_version {
>>> + u8 main_version;
>>> + u8 sub_version;
>>> + u8 test_version;
>>> +};
>>> +
>>> +struct qcom_ec_thermal_cap {
>>> +#define EC_THERMAL_FAN_CNT(x) (FIELD_GET(GENMASK(1, 0), (x)))
>>> +#define EC_THERMAL_FAN_TYPE(x) (FIELD_GET(GENMASK(4, 2), (x)))
>>> +#define EC_THERMAL_THERMISTOR_MASK(x) (FIELD_GET(GENMASK(7, 0), (x)))
>>> + u8 fan_cnt;
>>> + u8 fan_type;
>>> + u8 thermistor_mask;
>>> +};
>>> +
>>> +struct qcom_ec_cooling_dev {
>>> + struct thermal_cooling_device *cdev;
>>> + struct device *parent_dev;
>>> + u8 fan_id;
>>> + u8 state;
>>> +};
>>> +
>>> +struct qcom_ec {
>>> + struct qcom_ec_cooling_dev *ec_cdev;
>>> + struct qcom_ec_thermal_cap thermal_cap;
>>> + struct qcom_ec_version version;
>>> + struct i2c_client *client;
>>> +};
>>> +
>>> +static int qcom_ec_read(struct qcom_ec *ec, u8 cmd, u8 resp_len, u8 *resp)
>>> +{
>>> + int ret;
>>> +
>>> + ret = i2c_smbus_read_i2c_block_data(ec->client, cmd, resp_len, resp);
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> + else if (ret == 0 || ret == 0xff)
>>> + return -EOPNOTSUPP;
>>> +
>>> + if (resp[0] >= resp_len)
>>> + return -EINVAL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * EC Device Firmware Version:
>>> + *
>>> + * Read Response:
>>> + * ----------------------------------------------------------------------
>>> + * | Offset | Name | Description |
>>> + * ----------------------------------------------------------------------
>>> + * | 0x00 | Byte count | Number of bytes in response |
>>> + * | | | (excluding 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 |
>>> + * | | | (excluding byte count) |
>>> + * ------------------------------------------------------------------------------
>>> + * | 0x02 (LSB) | EC Thermal | Bit 0-1: Number of fans |
>>> + * | 0x3 | Capabilities | Bit 2-4: Type of fan |
>>
>> 0x03 ?
>>
>
> Typo, Will fix to 0x03 in next respin.
>
>>> + * | | | Bit 5-6: Reserved |
>>> + * | | | Bit 7: Data Valid/Invalid |
>>> + * | | | (Valid - 1, Invalid - 0) |
>>> + * | | | Bit 8-15: Thermistor 0 - 7 presence |
>>> + * | | | (1 present, 0 absent) |
>>> + * ------------------------------------------------------------------------------
>>> + *
>>> + */
>>> +static int qcom_ec_thermal_capabilities(struct device *dev)
>>> +{
>>> + struct i2c_client *client = to_i2c_client(dev);
>>> + struct qcom_ec *ec = i2c_get_clientdata(client);
>>> + struct qcom_ec_thermal_cap *cap = &ec->thermal_cap;
>>> + u8 resp[EC_THERMAL_CAP_RESP_LEN];
>>> + int ret;
>>> +
>>> + ret = qcom_ec_read(ec, EC_THERMAL_CAP_CMD, EC_THERMAL_CAP_RESP_LEN, resp);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + cap->fan_cnt = min(EC_MAX_FAN_CNT, EC_THERMAL_FAN_CNT(resp[1]));
>>> + cap->fan_type = EC_THERMAL_FAN_TYPE(resp[1]);
>>> + cap->thermistor_mask = EC_THERMAL_THERMISTOR_MASK(resp[2]);
>>> +
>>> + dev_dbg(dev, "Fan count: %d Fan Type: %d Thermistor Mask: %x\n",
>>
>> Please add include for dev_dbg().
>>
>> It seems you've missed at least some of my comments to v5, please recheck
>> those comments. I won't look further for now.
>>
>> --
>> i.
>>
>
> Added <linux/device.h> for dev_dbg() in v6, will include
> <linux/dev_printk.h> as well in in v8. That said, all your v5 comments
> have been addressed, here is a summary of what was fixed:
>
> - Add <linux/bits.h> for BIT()
> - Add <linux/err.h> for IS_ERR()
> - Switch thermistor mask format specifier from %d to %x
> - Add missing braces
> - Remove empty line within variable declarations
> - Change loop counter i to unsigned int
> - Replace snprintf() with scnprintf()
> - Use sizeof(name) instead of the EC_FAN_NAME_SIZE macro directly
> - Condense devm_thermal_of_cooling_device_register() to 2 lines
>
> Apologies if it appeared otherwise. If anything was missed, please do
> point it out and I will address it in the next respin.
>
Hi Ilpo,
Could you please take a look at this when you get a chance?
--
Best Regards,
Anvesh