Hi all, 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. > 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. > I disagree, I think it does not make much sense. This is already done by the NTC thermistor driver. The battery "subsystem" already provides operating-range-celsius and alert-celsius properties for that. Since the battery is linked to the AXP, all we need is to be able to ask the NTC thermistor driver to do the conversion from temperature to voltage of the two voltage values we get from the battery and use the result as threshold in the AXP registers. I wouldn't want to have the extrapolation done in two different places. I can see two ways of specifying that interation: battery -------------------> axp --------------------> ntc min/max °C request °C to V <-------------------- response V This however would require a phandle in the AXP to the NTC thermistor driver and I don't feel like it's that good of an idea? Another way would be to use the battery as a proxy for the voltage request to ntc. battery --------------------> axp min/max °C ntc <--------------- <-------------------- request °C to V request °C to V ---------------> ---------------------> response V response V This would require a phandle to the ntc thermistor in the battery node, which kind of makes sense to me. And since the AXP already has knowledge of the battery, it can request the appropriate value to the battery which then proxies it to and back from the ntc. Forgive me for my poor ASCII drawing skills :) hopefully it's good enough to convey my thoughts. > 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. > Since the ntc anyway needs this raw TS voltage and that patch does that, I think it's fine. Specifically, re-using this pin as a general-purpose ADC won't impact the current patchset. What we'll need is to have a pinctrl driver for the few pins in the AXP which have multiple functions. But that's outside of the scope of this patchset. Regarding the injected current, I don't have enough knowledge in electronics to understand how this will change things in the thermistor since in the NTC thermistor driver there's no logic related to the actual current being injected. Maybe it is related to some operating value required by the NTC? I can't say unfortunately. We can continue this discussion but I don't think this should block this patch as I don't see the outcome of this discussion change anything in this patchset. Reviewed-by: Quentin Schulz <foss+kernel@xxxxxxxxx> Thanks! Quentin