Re: [PATCH 2/8] platform: arm64: qcom-hamoa-ec: Add SoC junction temperature reporting

[Konrad Dybcio]

On 7/28/26 7:44 PM, Anvesh Jain P wrote:
> Add the EC command definitions and handler function for reporting the
> SoC junction temperature (Tj) to the EC.
> 
> Discover the platform's thermal sensor to zone mapping via the
> qcom,tsens device tree property, average the junction temperatures
> across the mapped zones, and periodically report the result to the EC
> over SMBus using a delayed work item. Serialize this and the existing
> EC command sequences (firmware version read, thermal capability read,
> SCI event control, and the SCI IRQ handler) under a new io_lock mutex,
> since the delayed work item now runs concurrently with those paths.
> 
> Re-arm the periodic report on resume and cancel it on suspend to avoid
> racing with the modern standby transition.
> ---

Missing sign-off, have you run b4 prep --check?

[...]


> +	mutex_lock(&ec->io_lock);
>  	ret = qcom_ec_read(ec, EC_FW_VERSION_CMD, EC_FW_VERSION_RESP_LEN, resp);
> +	mutex_unlock(&ec->io_lock);

Does it make more sense to simply stick a guard(mutex)(&ec->io_lock)
at the beginning of qcom_ec_read()?

[...]

> +static struct thermal_zone_device *
> +qcom_ec_sensor_to_zone(struct device_node *sensor_np, u32 sensor_id)
> +{
> +	struct device_node *tz_np __free(device_node) =
> +		of_find_node_by_name(NULL, "thermal-zones");
> +
> +	if (!tz_np)
> +		return ERR_PTR(-ENODEV);
> +
> +	for_each_available_child_of_node_scoped(tz_np, child) {
> +		struct of_phandle_args args;
> +
> +		if (of_parse_phandle_with_args(child, "thermal-sensors",
> +					       "#thermal-sensor-cells", 0, &args))
> +			continue;
> +
> +		of_node_put(args.np);
> +
> +		if (args.np == sensor_np &&
> +		    sensor_id == (args.args_count ? args.args[0] : 0))
> +			return thermal_zone_get_zone_by_name(child->name);
> +	}

I'm not sure that's the intended use of the API, but this is NHI
of_thermal_zone_find()

[...]

> +static void qcom_ec_sci_evt_disable(void *data)
> +{
> +	struct device *dev = data;
> +	int ret;
> +
> +	ret = qcom_ec_sci_evt_control(dev, false);
> +	if (ret < 0)
> +		dev_err(dev, "Failed to disable SCI events: %d\n", ret);
>  }

This should be a separate fix. FWIW suspending and resuming on
linux-next/master currently gives me:

[   43.754787] geni_i2c a84000.i2c: error turning SE resources:-13
[   43.754811] qcom-hamoa-ec 3-0076: Failed to read EC SCI Event: -13

Konrad