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

[Pablo Mazzini]

On Sun, Nov 8, 2026 at 2:50 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >     hwmon_dev = devm_hwmon_device_register_with_info(&adev->dev,
> > -                                                    "sy7636a_temperature", regmap,
> > +                                                    "sy7636a", regmap,
>
> This change is neither related to this patch nor necessary. Other drivers (unnecessarily)
> doing the same is not an argument.

Right, I'll drop it in v4.

On Sun, Nov 8, 2026 at 2:50 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> On Sun, Nov 08, 2026 at 09:36:01AM +0000, Pablo Mazzini wrote:
> > The driver enables VCOM at probe, which keeps the panel's high voltage
> > rails on for good. 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 | 37 +++++++++++++++++++++++------------
> >  1 file changed, 25 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hwmon/sy7636a-hwmon.c b/drivers/hwmon/sy7636a-hwmon.c
> > index a6b47452e29e..0c0601d47b87 100644
> > --- a/drivers/hwmon/sy7636a-hwmon.c
> > +++ b/drivers/hwmon/sy7636a-hwmon.c
> > @@ -9,26 +9,43 @@
> >   */
> >
> >  #include <linux/auxiliary_bus.h>
> > +#include <linux/bits.h>
> >  #include <linux/err.h>
> >  #include <linux/hwmon.h>
> >  #include <linux/module.h>
> >  #include <linux/regmap.h>
> > -#include <linux/regulator/machine.h>
> >
> > -#define SY7636A_REG_TERMISTOR_READOUT        0x08
> > +#define SY7636A_REG_FAULT_FLAG               0x07
> > +#define SY7636A_FAULT_FLAG_PG                BIT(0)
> >
> >  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;
> > +
> > +     /*
> > +      * The thermistor is sampled while ON_OFF is set, but the first sample
> > +      * is only ready after the ADC sampling and conversion time (ADC_SPL +
> > +      * ADC_DEL in the datasheet); until then it holds its reset value, 0.
> > +      * PG is only set once all rails are up, after the first sample, so
> > +      * read it in the same transfer and report the temperature only while
> > +      * it is set.
> > +      *
> > +      * With ON_OFF set but PG low (rails still ramping up, or a fault), no
> > +      * temperature is reported even if one may be available. That is
> > +      * accepted: the display driver reads the temperature only while it
> > +      * holds the rails on, after power good.
> > +      */
> > +     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;
> > +
> > +     *temp = (s8)buf[1] * 1000;
> >
> >       return 0;
> >  }
> > @@ -72,12 +89,8 @@ static int sy7636a_sensor_probe(struct auxiliary_device *adev,
> >       if (!regmap)
> >               return -ENODEV;
> >
> > -     err = devm_regulator_get_enable(&adev->dev, "vcom");
> > -     if (err)
> > -             return err;
> > -
> >       hwmon_dev = devm_hwmon_device_register_with_info(&adev->dev,
> > -                                                      "sy7636a_temperature", regmap,
> > +                                                      "sy7636a", regmap,
>
> This change is neither related to this patch nor necessary. Other drivers (unnecessarily)
> doing the same is not an argument.
>
> Guenter
>
> >                                                        &sy7636a_chip_info, NULL);
> >
> >       if (IS_ERR(hwmon_dev)) {