patch "iio: adc: axp288: Fix TS-pin handling" added to staging-linus
Subsystems (4)
This is a note to let you know that I've just added the patch titled
iio: adc: axp288: Fix TS-pin handling
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-linus branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From 9bcf15f75cac3c6a00d8f8083a635de9c8537799 Mon Sep 17 00:00:00 2001
From: Hans de Goede <[email protected]>
Date: Sat, 5 Jan 2019 19:36:18 +0100
Subject: iio: adc: axp288: Fix TS-pin handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Prior to this commit there were 3 issues with our handling of the TS-pin:
1) There are 2 ways how the firmware can disable monitoring of the TS-pin for designs which do not have a temperature-sensor for the battery:
a) Clearing bit 0 of the AXP20X_ADC_EN1 register
b) Setting bit 2 of the AXP288_ADC_TS_PIN_CTRL monitoring
Prior to this commit we were unconditionally setting both bits to the value used on devices with a TS. This causes the temperature protection to kick in on devices without a TS, such as the Jumper ezbook v2, causing them to not charge under Linux.
This commit fixes this by using regmap_update_bits when updating these 2 registers, leaving the 2 mentioned bits alone.
The next 2 problems are related to our handling of the current-source for the TS-pin. The current-source used for the battery temp-sensor (TS) is shared with the GPADC. For proper fuel-gauge and charger operation the TS current-source needs to be permanently on. But to read the GPADC we need to temporary switch the TS current-source to ondemand, so that the GPADC can use it, otherwise we will always read an all 0 value.
2) Problem 2 is we were writing hardcoded values to the ADC TS pin-ctrl register, overwriting various other unrelated bits. Specifically we were overwriting the current-source setting for the TS and GPIO0 pins, forcing it to 80ųA independent of its original setting. On a Chuwi Vi10 tablet this was causing us to get a too high adc value (due to a too high current-source) resulting in the following errors being logged:
ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] ACPI Error: Method parse/execution failed \_SB.SXP1._TMP, AE_ERROR
This commit fixes this by using regmap_update_bits to change only the relevant bits.
3) After reading the GPADC channel we were unconditionally enabling the TS current-source even on devices where the TS-pin is not used and the current-source thus was off before axp288_adc_read_raw call.
This commit fixes this by making axp288_adc_set_ts a nop on devices where the ADC is not enabled for the TS-pin.
--- drivers/iio/adc/axp288_adc.c | 76 ++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 16 deletions(-)
drivers/iio/adc/axp288_adc.c +60 -16
@@ -27,9 +27,18 @@#include <linux/iio/machine.h> #include <linux/iio/driver.h> -#define AXP288_ADC_EN_MASK 0xF1 -#define AXP288_ADC_TS_PIN_GPADC 0xF2 -#define AXP288_ADC_TS_PIN_ON 0xF3 +/* +* This mask enables all ADCs except for the battery temp-sensor (TS), that is +* left as-is to avoid breaking charging on devices without a temp-sensor. +*/ +#define AXP288_ADC_EN_MASK 0xF0 +#define AXP288_ADC_TS_ENABLE 0x01 + +#define AXP288_ADC_TS_CURRENT_ON_OFF_MASK GENMASK(1, 0) +#define AXP288_ADC_TS_CURRENT_OFF (0 << 0) +#define AXP288_ADC_TS_CURRENT_ON_WHEN_CHARGING (1 << 0) +#define AXP288_ADC_TS_CURRENT_ON_ONDEMAND (2 << 0) +#define AXP288_ADC_TS_CURRENT_ON (3 << 0)enum axp288_adc_id {AXP288_ADC_TS ,
@@ -44,6 +53,7 @@ enum axp288_adc_id {struct axp288_adc_info {int irq;struct regmap *regmap; +bool ts_enabled; };static const struct iio_chan_spec axp288_adc_channels[] = {
@@ -115,21 +125,33 @@ static int axp288_adc_read_channel(int *val, unsigned long address,return IIO_VAL_INT ; } -static int axp288_adc_set_ts (struct regmap *regmap,unsigned int mode, -unsigned long address) +/* +* The current-source used for the battery temp-sensor (TS) is shared +* with the GPADC. For proper fuel-gauge and charger operation the TS +* current-source needs to be permanently on. But to read the GPADC we +* need to temporary switch the TS current-source to ondemand, so that +* the GPADC can use it, otherwise we will always read an all 0 value. +*/ +static int axp288_adc_set_ts (struct axp288_adc_info *info, +unsigned int mode,unsigned long address) {int ret; -/* channels other than GPADC do not need to switch TS pin */ +/* No need to switch the current-source if the TS pin is disabled */ +if (!info->ts_enabled) +return 0 ; + +/* Channels other than GPADC do not need the current source */ if (address !=AXP288_GP_ADC_H )return 0 ; - ret =regmap_write (regmap,AXP288_ADC_TS_PIN_CTRL , mode); + ret =regmap_update_bits (info->regmap,AXP288_ADC_TS_PIN_CTRL , +AXP288_ADC_TS_CURRENT_ON_OFF_MASK , mode);if (ret)return ret;/* When switching to the GPADC pin give things some time to settle */ -if (mode ==AXP288_ADC_TS_PIN_GPADC ) +if (mode ==AXP288_ADC_TS_CURRENT_ON_ONDEMAND )usleep_range (6000 ,10000 );return 0 ;
@@ -145,14 +167,14 @@ static int axp288_adc_read_raw(struct iio_dev *indio_dev,mutex_lock (&indio_dev->mlock);switch (mask) {case IIO_CHAN_INFO_RAW : -if (axp288_adc_set_ts (info->regmap,AXP288_ADC_TS_PIN_GPADC , +if (axp288_adc_set_ts (info,AXP288_ADC_TS_CURRENT_ON_ONDEMAND , chan->address)) {dev_err (&indio_dev->dev,"GPADC mode\n" ); ret = -EINVAL ;break ; } ret =axp288_adc_read_channel (val, chan->address, info->regmap); -if (axp288_adc_set_ts (info->regmap,AXP288_ADC_TS_PIN_ON , +if (axp288_adc_set_ts (info,AXP288_ADC_TS_CURRENT_ON , chan->address))dev_err (&indio_dev->dev,"TS pin restore\n" );break ;
@@ -164,13 +186,35 @@ static int axp288_adc_read_raw(struct iio_dev *indio_dev,return ret; } -static int axp288_adc_set_state (struct regmap *regmap) +static int axp288_adc_initialize (struct axp288_adc_info *info) { -/* ADC should be always enabled for internal FG to function */ -if (regmap_write (regmap,AXP288_ADC_TS_PIN_CTRL ,AXP288_ADC_TS_PIN_ON )) -return -EIO ; +int ret, adc_enable_val; + +/* +* Determine if the TS pin is enabled and set the TS current-source +* accordingly. +*/ + ret =regmap_read (info->regmap,AXP20X_ADC_EN1 , &adc_enable_val); +if (ret) +return ret; + +if (adc_enable_val &AXP288_ADC_TS_ENABLE ) { + info->ts_enabled = true; + ret =regmap_update_bits (info->regmap,AXP288_ADC_TS_PIN_CTRL , +AXP288_ADC_TS_CURRENT_ON_OFF_MASK , +AXP288_ADC_TS_CURRENT_ON ); + }else { + info->ts_enabled = false; + ret =regmap_update_bits (info->regmap,AXP288_ADC_TS_PIN_CTRL , +AXP288_ADC_TS_CURRENT_ON_OFF_MASK , +AXP288_ADC_TS_CURRENT_OFF ); + } +if (ret) +return ret; -return regmap_write (regmap,AXP20X_ADC_EN1 ,AXP288_ADC_EN_MASK ); +/* Turn on the ADC for all channels except TS, leave TS as is */ +return regmap_update_bits (info->regmap,AXP20X_ADC_EN1 , +AXP288_ADC_EN_MASK ,AXP288_ADC_EN_MASK ); }static const struct iio_info axp288_adc_iio_info = {
@@ -200,7 +244,7 @@ static int axp288_adc_probe(struct platform_device *pdev)* Set ADC to enabled state at all time, including system suspend. * otherwise internal fuel gauge functionality may be affected. */ - ret =axp288_adc_set_state (axp20x->regmap); + ret =axp288_adc_initialize (info);if (ret) {dev_err (&pdev->dev,"unable to enable ADC device\n" );return ret;
Signature · 1 line
2.20.1