Re: [PATCH v2 5/7] hwmon: sy7636a: read the temperature only while the rails are on
From: Guenter Roeck
Date: Mon Nov 02 2026 - 10:19:22 EST
On 11/2/26 02:25, Andreas Kemnade wrote:
On Sun, 1 Nov 2026 19:35:16 -0800As mentioned in the other mail, downside is that the temperature will not
Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 11/1/26 14:31, Pablo Mazzini wrote:That is what I read from there, too. But the PG bit will only
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, ®_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.
be set if the On/Off bit is also set.
The datasheet does suggest that temperatures are only measuredI can confirm from tests that temperature reads zero if regulator
if bit 7 of the operation mode register is set, but that is
different from the "power good" bit in the fault register.
was not yet powered on.
I read the datasheet in that way so things happen it that order:
1. on bit is set
2. ramping up voltages start / temperature sampling starts.
3. after some time (sampling time + conversion time (ADC_SPL+ADC_DEL)?),
the first sample is ready and the temperature can be read.
4. after some more time voltage lines stabilize
5. after 1.5 ms PG is asserted
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.
That should definitively be commented in the code. I have pointed out
races earlier. This has of course the drawback that it limits the time
frame in which reads are possible a bit. I doubt it is worth dealing with
synchronisation code to use that on/off bit (although posssible).
be reported if EN=1 and PG=0 (for whatever reason), even though
the temperature may be available. At the very least the reason for
accepting this limitation will need to be explained in the driver.
Guenter