[PATCH] iio: adc: axp288: Fix TS-pin handling

[PATCH] iio: adc: axp288: Fix TS-pin handling

[Hans de Goede]

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.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1610545
Fixes: 3091141d7803 ("iio: adc: axp288: Fix the GPADC pin ...")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/adc/axp288_adc.c | 74 ++++++++++++++++++++++++++++--------
 1 file changed, 58 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
index 031d568b4972..99a6b804fd49 100644
--- a/drivers/iio/adc/axp288_adc.c
+++ b/drivers/iio/adc/axp288_adc.c
@@ -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,33 @@ 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 if it is not enabled,
+	 * turn the TS current-source off, so that the shared current-source
+	 * will be available for the GPADC.
+	 */
+	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;
+	} 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 +242,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;
-- 
2.19.1

Re: [PATCH] iio: adc: axp288: Fix TS-pin handling

[Jonathan Cameron]

On Fri,  4 Jan 2019 23:13:05 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> 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.
> 
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1610545
> Fixes: 3091141d7803 ("iio: adc: axp288: Fix the GPADC pin ...")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

To me this looks fine (really minor comment inline).

I'll just wait until rc1 is out so as to have a suitable base to gather
a few fixes on before applying this.

Give me a poke if I seem to have forgotten in a week or so!
Will mark it for stable when I apply.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/axp288_adc.c | 74 ++++++++++++++++++++++++++++--------
>  1 file changed, 58 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
> index 031d568b4972..99a6b804fd49 100644
> --- a/drivers/iio/adc/axp288_adc.c
> +++ b/drivers/iio/adc/axp288_adc.c
> @@ -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,33 @@ 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 if it is not enabled,
> +	 * turn the TS current-source off, so that the shared current-source
> +	 * will be available for the GPADC.
> +	 */
> +	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;
> +	} else {
> +		info->ts_enabled = false;
Really minor but I'd have found this clearer as 
	info->ts_enabled = adc_enable_val & AXP288_ADC_TS_ENABLE;
	if (info->ts_enabled) { ...

> +		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 +242,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;

Re: [PATCH] iio: adc: axp288: Fix TS-pin handling

[Hans de Goede]

Hi,

On 05-01-19 17:31, Jonathan Cameron wrote:
> On Fri,  4 Jan 2019 23:13:05 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> 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.
>>
>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1610545
>> Fixes: 3091141d7803 ("iio: adc: axp288: Fix the GPADC pin ...")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> To me this looks fine (really minor comment inline).
> 
> I'll just wait until rc1 is out so as to have a suitable base to gather
> a few fixes on before applying this.

Thank you for the review. Note I noticed one small oversight
this morning, we no longer set the TS current-source to on on probe when
the TS pin is enabled. In practice it seems the firmware always sets the
TS pin to on in this case, but better safe then sorry and we did explictly
set the TS current-source to on before this patch.

I've prepared a v2 fixing this.

Regards,

Hans

>> ---
>>   drivers/iio/adc/axp288_adc.c | 74 ++++++++++++++++++++++++++++--------
>>   1 file changed, 58 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
>> index 031d568b4972..99a6b804fd49 100644
>> --- a/drivers/iio/adc/axp288_adc.c
>> +++ b/drivers/iio/adc/axp288_adc.c
>> @@ -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,33 @@ 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 if it is not enabled,
>> +	 * turn the TS current-source off, so that the shared current-source
>> +	 * will be available for the GPADC.
>> +	 */
>> +	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;
>> +	} else {
>> +		info->ts_enabled = false;
> Really minor but I'd have found this clearer as
> 	info->ts_enabled = adc_enable_val & AXP288_ADC_TS_ENABLE;
> 	if (info->ts_enabled) { ...
> 
>> +		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 +242,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;
>