Re: [PATCH v2 5/7] hwmon: sy7636a: read the temperature only while the rails are on

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hwmon: sy7636a: returning -ENODATA when rails are off disables thermal zone
--

commit 72fc8d76dc4f0fe2ee1ec4e02805a6f20803ef65
Author: Pablo Mazzini <pmazzini@gmail.com>

hwmon: sy7636a: read the temperature only while the rails are on

This commit stops enabling the VCOM regulator at probe, registers the hwmon
device under the chip name "sy7636a", and reads the fault flag alongside the
thermistor in a single bulk transfer, returning -ENODATA when power good is
not set.

> diff --git a/drivers/hwmon/sy7636a-hwmon.c b/drivers/hwmon/sy7636a-hwmon.c
> index f01f368d68e5f..475aff997c2d2 100644
> --- a/drivers/hwmon/sy7636a-hwmon.c
> +++ b/drivers/hwmon/sy7636a-hwmon.c
> @@ -14,7 +14,6 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> -#include <linux/regulator/machine.h>
>  
>  #include <linux/mfd/sy7636a.h>
>  
> @@ -22,14 +21,22 @@ static int sy7636a_read(struct device *dev, enum hwmon_sensor_types type,
>  			u32 attr, int channel, long *temp)
>  {
>  	struct regmap *regmap = dev_get_drvdata(dev);
> -	int ret, reg_val;
> -
> -	ret = regmap_read(regmap,
> -			  SY7636A_REG_TERMISTOR_READOUT, &reg_val);
> +	u8 buf[2];
> +	int ret;
> +
> +	/*
> +	 * Read power good and the thermistor in one transfer: the thermistor is
> +	 * only sampled while the rails are on, and power good comes after the
> +	 * first sample
> +	 */
> +	ret = regmap_bulk_read(regmap, SY7636A_REG_FAULT_FLAG, buf, sizeof(buf));
>  	if (ret)
>  		return ret;
>  
> -	*temp = (s8)reg_val * 1000;
> +	if (!(buf[0] & SY7636A_FAULT_FLAG_PG))
> +		return -ENODATA;

[Severity: High]
Will returning -ENODATA in sy7636a_read() when SY7636A_FAULT_FLAG_PG is
cleared cause the thermal core to permanently disable the attached thermal
zone when the rails remain off?

In drivers/hwmon/sy7636a-hwmon.c, sy7636a_info sets HWMON_C_REGISTER_TZ:

static const struct hwmon_channel_info * const sy7636a_info[] = {
	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
	NULL
};

In arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts, the epd-thermal zone
binds to &sy7636a with a 30-second polling delay while the vcom regulator only
specifies regulator-boot-on:

	thermal-zones {
		epd-thermal {
			thermal-sensors = <&sy7636a>;
			polling-delay-passive = <30000>;
			polling-delay = <30000>;

When the thermal zone is polled while the EPD rails are off, the call chain
__thermal_zone_device_update() -> __thermal_zone_get_temp() ->
hwmon_thermal_get_temp() -> sy7636a_read() passes -ENODATA to
thermal_zone_recheck() in drivers/thermal/thermal_core.c:

static void thermal_zone_recheck(struct thermal_zone_device *tz, int error)
{
	if (error == -EAGAIN) {
		thermal_zone_device_set_polling(tz, THERMAL_RECHECK_DELAY);
		return;
	}

	if (tz->recheck_delay_jiffies == THERMAL_RECHECK_DELAY)
		dev_info(&tz->device, "Temperature check failed (%d)\n", error);

	thermal_zone_device_set_polling(tz, tz->recheck_delay_jiffies);

	tz->recheck_delay_jiffies += max(tz->recheck_delay_jiffies >> 1, 1ULL);
	if (tz->recheck_delay_jiffies > THERMAL_MAX_RECHECK_DELAY) {
		thermal_zone_broken_disable(tz);

Because thermal_zone_recheck() only treats -EAGAIN as a non-fatal retry and
treats -ENODATA as a sensor failure, it logs an error, switches to
exponential backoff polling starting at 250 ms, and after roughly 5.5 minutes
with the rails off calls thermal_zone_broken_disable(), which sets the thermal
zone mode to THERMAL_DEVICE_DISABLED so thermal monitoring remains disabled
even after the rails are turned back on.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101223125.302001-1-pmazzini@gmail.com?part=5