Hi Evgeny,
On Tue, Nov 30, 2021 at 02:58:23AM +0300, Evgeny Boger wrote:
> (added linux-pm@ list and maintainers)
>
>
> Actually, on second though, I think it might be doable to add voltage to
> temperature conversion to this driver.
>
> I think since the NTC thermistor belongs to the battery, not charger, the
> thermistor should be described in monitored battery node.
Then we would duplicate the code in ntc thermistor driver and battery
subsystem (or simple-battery driver I assume), instead of reusing the
ntc thermistor driver.
> So I propose to extend battery node (power/supply/battery.yaml) by adding
> something like:
>
> thermistor-resistance-temp-table = <25 10000>, <35 6530>, ...;
>
> This driver will then interpolate between points to report temperature.
>
Not sure this makes sense, that's the point of the ntc thermistor driver
which does all of this already AFAICT.
The battery node already supports operating-range-celsius and
alert-celsius. This driver should read that and then ask the ntc
thermistor driver what's the voltage of the thermistor associated with
this temperature and then set the register to this value.
What's missing in the ntc thermistor driver and/or its subsystem is the
ability to request a specific temperature to voltage conversion,
unrelated to the current value of the NTC thermistor.
In my head I picture the following:
battery node ----------> axp -------------------------------> ntc
max/min °C request °C to V for max/min
<-------------------------------
V for max/min °C
The issue I see in this is that the axp needs to have a phandle to the
ntc thermistor... which does not make much sense from DTS point of view
IMO.
Now.. we could also have the following instead by extending the battery
subsystem:
battery node --------------> axp
max/min °C
ntc <---------- <-------------
request °C to V request °C to V
----------> -------------->
V for max/min °C V for max/min °C
which means the battery driver/susbystem would have a phandle to the ntc
thermistor and proxy the °C to V conversionr equest from axp to ntc and
the answer back to the axp.
Obviously, ntc would still be a consumer of axp ts iio channel to
report the battery temp to userspace.
> We can also adjust PMIC voltage thresholds based on this table and
> "alert-celsius" property already described in battery.yaml.
>
> I think the driver should report raw TS voltage as well, because the TS pin
> can also be used as general-purpose ADC pin.
>
Raw TS voltage is still reported with your first patch so that's fine.
We'd need to create a pinctrl driver for the AXP to handle the few pins
with multiple fonctions, but it's outside of the scope of this patch
series :)
Regarding the injected current, I don't have enough knowledge in
electronics to understand what exactly would happen to the NTC
thermistor here since it seems the current is not used anywhere in the
formula to get the ohm of the resistor which is then converted to the
actual temperature. At least in the ntc thermistor driver. Also not sure
where this injected current should be declared in the device tree, is it
a limitation of the NTC thermistor (some kind of operating range?) which
should be propagated to the AXP to make it inject more/less current
depending on the NTC thermistor?
This patch is fine in itself since it's required for doing anything more
that is currently discussed in this thread. We can continue discussing
this but I don't think it is preventing this patch to continue its
normal life and be merged :)
Reviewed-by: Quentin Schulz <foss+kernel@xxxxxxxxx>
Cheers,
Quentin
