Re: [PATCH 1/2] iio:adc:axp20x: add support for NTC thermistor

Quentin Schulz
2021-12-01T10:42:24Z

Hi Evgeny,

On Thu, Nov 18, 2021 at 05:12:32PM +0300, Evgeny Boger wrote:
> Most AXPxxx-based reference designs place a 10k NTC thermistor on a
> TS pin. When appropriately configured, AXP PMICs will inject fixed
> current (80uA by default) into TS pin and measure the voltage across a
> thermistor. The PMIC itself will by default compare this voltage with
> predefined thresholds and disable battery charging whenever

They actually are configurable, we just don't have the knobs for this in
the kernel.

"In the diagram above, VTH/VTL refers to the high temperature threshold
and low temperature threshold, which is programmable via registers
REG38H/39H/3CH/3DH respectively. " in AXP209 datasheet, section
"Battery temperature detection".

> the battery is too hot or too cold.
>
> Alternatively, the TS pin can be configured as general-purpose
> ADC input. This mode is not supported by the driver.
>
> This patch allows reading the voltage on the TS pin. It can be then
> either processed by userspace or used by kernel consumer like hwmon
> ntc thermistor driver.
>
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
> drivers/iio/adc/axp20x_adc.c | 45 +++++++++++++++++++++++++++++++-----
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 3e0c0233b431..12d469a52cea 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
[...]
> +static int axp22x_adc_scale_voltage(int channel, int *val, int *val2)
> +{
> + switch (channel) {
> + case AXP22X_BATT_V:
> + /* 1.1 mV per LSB */
> + *val = 1;
> + *val2 = 100000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case AXP22X_TS_IN:
> + /* 0.8 mV per LSB */
> + *val = 0;
> + *val2 = 800000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> static int axp813_adc_scale_voltage(int channel, int *val, int *val2)
> {
> switch (channel) {
[...]
> @@ -378,12 +415,7 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
> {
> switch (chan->type) {
> case IIO_VOLTAGE:
> - if (chan->channel != AXP22X_BATT_V)
> - return -EINVAL;
> -
> - *val = 1;
> - *val2 = 100000;
> - return IIO_VAL_INT_PLUS_MICRO;
> + return axp22x_adc_scale_voltage(chan->channel, val, val2);
>

I would have personally split the move to a separate
axp22x_adc_scale_voltage function in a separate commit. I was a bit
confused at first why in the diff above there was a AXP22X_BATT_V
addition since this commit is about AXP22X_TS_IN.

If the maintainers are ok with it, I don't mind too much.

I don't have the HW to test this, but changes look ok.

Reviewed-by: Quentin Schulz <foss+kernel@0leil.net>

Thanks!
Quentin