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

[Guenter Roeck]

On 11/1/26 14:31, Pablo Mazzini wrote:
> The driver enables VCOM at probe, which keeps the panel's high voltage
> rails on for good, and gets a dummy regulator if it probes before the
> regulator exists. Read the temperature only while the rails are on, when
> the chip samples it, and return -ENODATA otherwise. Read power good with
> it in one transfer, so the value can not be from before the first sample
> or after the rails went off. Register it under the chip name, as the
> TPS65185 and FP9931 drivers do.
>
> Signed-off-by: Pablo Mazzini <pmazzini@xxxxxxxxx>
> ---
>   drivers/hwmon/sy7636a-hwmon.c | 25 ++++++++++++++-----------
>   1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwmon/sy7636a-hwmon.c b/drivers/hwmon/sy7636a-hwmon.c
> index f01f368d68e5..475aff997c2d 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;
> +

I had a look at the datasheet. It does not say anything about the
temperature not being measured when PG is not set.

The datasheet does suggest that temperatures are only measured
if bit 7 of the operation mode register is set, but that is
different from the "power good" bit in the fault register.

So why the association with the PG bit and not with the On/Off bit
of the operation mode register ? That warrants some more explanation.

Thanks,
Guenter
 > +	*temp = (s8)buf[1] * 1000;
>   
>   	return 0;
>   }
> @@ -72,12 +79,8 @@ static int sy7636a_sensor_probe(struct platform_device *pdev)
>   	if (!regmap)
>   		return -EPROBE_DEFER;
>   
> -	err = devm_regulator_get_enable(&pdev->dev, "vcom");
> -	if (err)
> -		return err;
> -
>   	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> -							 "sy7636a_temperature", regmap,
> +							 "sy7636a", regmap,
>   							 &sy7636a_chip_info, NULL);
>   
>   	if (IS_ERR(hwmon_dev)) {