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

[Pablo Mazzini]

Thanks for looking at it.

Reading the temperature races with the rails being switched on and
off, as Andreas pointed out on v1.

PG only goes high once all rails are up, after the first measurement.
It is also in register 0x07, next to the thermistor readout, so one
short transfer reads both.

Pablo

On Mon, Nov 2, 2026 at 3:35 AM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> 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)) {
>