Re: [PATCH 4/6] thermal: qcom: tsens: add hardware-bin trip-point filtering

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] thermal/tsens: tsens_probe swallows -EPROBE_DEFER permanently disabling hw-bin
- [Low] kconfig: QCOM_TSENS selects THERMAL_BIN without THERMAL_OF dependency
--

commit 8bed8cb744b82aeda728860cd801b8d298e30e15
Author: Amit Kucheria <amit.kucheria@oss.qualcomm.com>

thermal: qcom: tsens: add hardware-bin trip-point filtering

This commit introduces hardware-bin trip-point filtering for TSENS sensors
on Qualcomm SoCs. It adds a struct tsens_hw_bin_desc to read manufacturing
bins from nvmem cells, synthesizing these into hw-version bitfields that
the thermal core can use to filter supported hardware bins.

> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index a8cf7e2582010..6aab493333121 100644
> --- a/drivers/thermal/qcom/Kconfig
> +++ b/drivers/thermal/qcom/Kconfig
> @@ -3,6 +3,7 @@ config QCOM_TSENS
>  	tristate "Qualcomm TSENS Temperature Alarm"
>  	depends on NVMEM_QCOM_QFPROM
>  	depends on ARCH_QCOM || COMPILE_TEST
> +	select THERMAL_BIN

[Severity: Low]
Does selecting THERMAL_BIN here without inheriting a dependency on THERMAL_OF
cause an unmet direct dependency warning?

If the kernel is configured with COMPILE_TEST enabled but OF/THERMAL_OF
disabled, this select forces THERMAL_BIN on, violating its explicit
dependency on THERMAL_OF. This can lead to build failures under WERROR.

>  	help
>  	  This enables the thermal sysfs driver for the TSENS device. It shows

[ ... ]

> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index b5ec70201e2f3..26ba63da7163f 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c

[ ... ]

> @@ -1425,6 +1602,12 @@ static int tsens_probe(struct platform_device *pdev)
>  	priv->feat = data->feat;
>  	priv->fields = data->fields;
>  
> +	ret = tsens_populate_hw_bin_info(priv, data);
> +	if (ret)
> +		dev_warn(&pdev->dev,
> +			 "thermal-bin failed to read FUSE data (%d): trips with temperature-bin will use first entry\n",
> +			 ret);

[Severity: Medium]
Does catching all errors here permanently disable hardware binning if the
nvmem provider is not yet ready?

If the nvmem provider (qfprom) probes after tsens,
nvmem_cell_read_variable_le_u32() in tsens_populate_hw_bin_info() will
return -EPROBE_DEFER. By logging a warning and continuing instead of
returning the error to the driver core, the probe completes and the
fallback state with default trip points becomes permanent.

Should this explicitly check for and propagate -EPROBE_DEFER?

> +
>  	platform_set_drvdata(pdev, priv);
>  
>  	device_init_wakeup(dev, !data->no_irq_wake);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-thermal_bin-v1-0-5a29be7e8ec5@oss.qualcomm.com?part=4