On 14/12/16 13:55, Hans de Goede wrote:
> For some reason the axp288_adc driver was modifying the
> AXP288_ADC_TS_PIN_CTRL register, changing bits 0-1 depending on
> whether the GP_ADC channel or another channel was written.
>
> These bits control when a bias current is send to the TS_PIN, the
> GP_ADC has its own pin and a separate bit in another register to
> control the bias current.
>
> Not only does changing when to enable the TS_PIN bias current
> (always or only when sampling) when reading the GP_ADC make no sense
> at all, the code is modifying these bits is writing the entire register,
> assuming that all the other bits have their default value.
>
> So if the firmware has configured a different bias-current for either
> pin, then that change gets clobbered by the write, likewise if the
> firmware has set bit 2 to indicate that the battery has no thermal sensor,
> this will get clobbered by the write.
>
> This commit fixes all this, by simply removing all writes to the
> AXP288_ADC_TS_PIN_CTRL register, they are not needed to read the
> GP_ADC pin, and can actually be harmful.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
I guess you probably have more up to date contact details than I do,
but seems worth trying to cc Jacob Pan on this to see if we can find
out what the original reasoning behind this was. Seems a very odd
thing to do with no purpose!
If Jacob isn't contactable we'll fall back to guessing it was just
an oddity of driver evolution.
Jonathan
> ---
> drivers/iio/adc/axp288_adc.c | 32 +-------------------------------
> 1 file changed, 1 insertion(+), 31 deletions(-)
>
> diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
> index 7fd2494..64799ad 100644
> --- a/drivers/iio/adc/axp288_adc.c
> +++ b/drivers/iio/adc/axp288_adc.c
> @@ -28,8 +28,6 @@
> #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
>
> enum axp288_adc_id {
> AXP288_ADC_TS,
> @@ -123,16 +121,6 @@ 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)
> -{
> - /* channels other than GPADC do not need to switch TS pin */
> - if (address != AXP288_GP_ADC_H)
> - return 0;
> -
> - return regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, mode);
> -}
> -
> static int axp288_adc_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -143,16 +131,7 @@ 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,
> - 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,
> - chan->address))
> - dev_err(&indio_dev->dev, "TS pin restore\n");
> break;
> default:
> ret = -EINVAL;
> @@ -162,15 +141,6 @@ static int axp288_adc_read_raw(struct iio_dev *indio_dev,
> return ret;
> }
>
> -static int axp288_adc_set_state(struct regmap *regmap)
> -{
> - /* 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;
> -
> - return regmap_write(regmap, AXP20X_ADC_EN1, AXP288_ADC_EN_MASK);
> -}
> -
> static const struct iio_info axp288_adc_iio_info = {
> .read_raw = &axp288_adc_read_raw,
> .driver_module = THIS_MODULE,
> @@ -199,7 +169,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 = regmap_write(info->regmap, AXP20X_ADC_EN1, AXP288_ADC_EN_MASK);
> if (ret) {
> dev_err(&pdev->dev, "unable to enable ADC device\n");
> return ret;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
