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@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 @@
>>>>> [...]
>>>>> +/*
>>>>> + * 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