Re: [PATCH 5/7] regulator: sy7636a: report the temperature
From: Andreas Kemnade
Date: Mon Oct 26 2026 - 19:28:54 EST
On Mon, 26 Oct 2026 07:35:24 -0700
Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> On Mon, Oct 26, 2026 at 12:39:16AM +0000, Pablo Mazzini wrote:
> > The separate hwmon cell enables VCOM at probe. Report the temperature
> > from here, as the TPS65185 and FP9931 drivers do, and under the chip
> > name as they do: read it while the rails are on and return -ENODATA
> > otherwise. Boards that need the rails on mark VCOM always-on. Drop the
> > hwmon driver.
>
> This is going the wrong way. The correct solution would be to convert
> the hwmon driver to an auxiliary driver. I am not going to approve
> this patch. I am not giving an official NACK (yet).
>
First we need the problem defined, then the solution, and then we can
talk about what is the correct solution. The commit message does not
name a problem.
So what we have:
one bit to turn on a bunch of things,
after one time to sample (< 1ms), fresh values are provided on the ADC
register periodically.
after some more time (max. 50ms), several voltage outputs are ready.
Powering on those things is expensive, so you would not use this as an
ordinary temperature sensor.
The problems I see in the sensor driver this patch might want to solve:
- problems if the regulator driver is registered earlier.
- not having powered on the regulator all the time (that is described
in the cover letter)
The way the patch handles it does not require any structural change,
availabitily of the regmap is given. So it would be a separate cleanup
patch.
But it introduces another race condition.
The temperature might be read too early.
1ms delay should suffice.
The SY7636A_OPERATION_MODE_CRL_ONOFF flag might be reset before
reading the temperature. If that register does not reset to zero
but just keeps the temperature last read, then we have no problem.
If not, there needs to be some synchronisation between the regulator
part and the sensor part, probably a mutex saved in a common structure.
And in that case, structural change of the drivers is needed which
I see the following possibilities:
a) non-simple mfd driver
b) aux device as a child of the regulator driver.
c) regulator driver containing everything.
No strong personal opinion here.
Regards,
Andreas