Re: [PATCH V3 2/5] platform: arm64: Add driver for EC found on Qualcomm reference devices
From: Sibi Sankar
Date: Mon Mar 09 2026 - 06:04:35 EST
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 qualcommThis mutex looks redundant to me for the current implementation. You
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>
---
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);
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.
[...]I think it's nice to provide users a way to override the fan speed, but
+/*
+ * 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);
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