| Message ID | 20211118141233.247907-2-boger@wirenboard.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: axp20x: add support for NTC thermistor | expand |
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 > 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> Acked-by: Maxime Ripard <maxime@cerno.tech> Maxime
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, it's just that we don't have the means to configure it currently from 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 the 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; > + } > +} [...] > @@ -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 actually have made the move to axp22x_adc_scale_voltage function for AXP22X_BATT_V channel separate from this commit because I was a bit confused in the original review why suddenly there was an addition for AXP22X_BATT_V while this patch is about AXP22X_TS_IN. If maintainers are ok with this, I don't mind. I don't have any HW to test this but the changes make sense to me: Reviewed-by: Quentin Schulz <foss+kernel@0leil.net> Thanks! Quentin
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
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 @@ -186,6 +186,8 @@ static const struct iio_chan_spec axp20x_adc_channels[] = { AXP20X_BATT_CHRG_I_H), AXP20X_ADC_CHANNEL(AXP20X_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT, AXP20X_BATT_DISCHRG_I_H), + AXP20X_ADC_CHANNEL(AXP20X_TS_IN, "ts_v", IIO_VOLTAGE, + AXP20X_TS_IN_H), }; static const struct iio_chan_spec axp22x_adc_channels[] = { @@ -203,6 +205,8 @@ static const struct iio_chan_spec axp22x_adc_channels[] = { AXP20X_BATT_CHRG_I_H), AXP20X_ADC_CHANNEL(AXP22X_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT, AXP20X_BATT_DISCHRG_I_H), + AXP20X_ADC_CHANNEL(AXP22X_TS_IN, "ts_v", IIO_VOLTAGE, + AXP22X_TS_ADC_H), }; static const struct iio_chan_spec axp813_adc_channels[] = { @@ -222,6 +226,8 @@ static const struct iio_chan_spec axp813_adc_channels[] = { AXP20X_BATT_CHRG_I_H), AXP20X_ADC_CHANNEL(AXP22X_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT, AXP20X_BATT_DISCHRG_I_H), + AXP20X_ADC_CHANNEL(AXP813_TS_IN, "ts_v", IIO_VOLTAGE, + AXP288_TS_ADC_H), }; static int axp20x_adc_raw(struct iio_dev *indio_dev, @@ -307,11 +313,36 @@ static int axp20x_adc_scale_voltage(int channel, int *val, int *val2) *val2 = 400000; return IIO_VAL_INT_PLUS_MICRO; + case AXP20X_TS_IN: + /* 0.8 mV per LSB */ + *val = 0; + *val2 = 800000; + return IIO_VAL_INT_PLUS_MICRO; + default: return -EINVAL; } } +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) { @@ -325,6 +356,12 @@ static int axp813_adc_scale_voltage(int channel, int *val, int *val2) *val2 = 100000; return IIO_VAL_INT_PLUS_MICRO; + case AXP813_TS_IN: + /* 0.8 mV per LSB */ + *val = 0; + *val2 = 800000; + return IIO_VAL_INT_PLUS_MICRO; + default: return -EINVAL; } @@ -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); case IIO_CURRENT: *val = 0; @@ -488,6 +520,7 @@ static int axp22x_read_raw(struct iio_dev *indio_dev, { switch (mask) { case IIO_CHAN_INFO_OFFSET: + /* For PMIC temp only */ *val = -2677; return IIO_VAL_INT;
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 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(-)