Re: [PATCH 2/2] dt-bindings: iio: adc: document TS voltage in AXP PMICs

[Quentin Schulz <foss+kernel@0leil.net>]

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@0leil.net>

Thanks!
Quentin