linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Jonathan Cameron <jic23@kernel.org>, Chen-Yu Tsai <wens@csie.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
	"russianneuromancer @ ya . ru" <russianneuromancer@ya.ru>,
	linux-iio@vger.kernel.org, linux-i2c@vger.kernel.org,
	Hans de Goede <hdegoede@redhat.com>
Subject: [PATCH] iio: adc: axp288: Drop bogus AXP288_ADC_TS_PIN_CTRL register modifications
Date: Wed, 14 Dec 2016 14:55:25 +0100	[thread overview]
Message-ID: <20161214135525.16477-1-hdegoede@redhat.com> (raw)

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@redhat.com>
---
 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;
-- 
2.9.3

             reply	other threads:[~2016-12-14 13:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-14 13:55 Hans de Goede [this message]
2016-12-14 15:00 ` [PATCH] iio: adc: axp288: Drop bogus AXP288_ADC_TS_PIN_CTRL register modifications Chen-Yu Tsai
2016-12-30 16:46 ` Jonathan Cameron
     [not found]   ` <1f31b2e7-90fa-0fb8-5f6e-a8ee2ddf69f7-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-12-30 18:15     ` Jacob Pan
2017-01-01 11:19       ` Hans de Goede
     [not found]         ` <054b52de-40fd-a879-7b8e-2d25fed18840-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-01-03 18:23           ` Jacob Pan
2017-01-03 22:10             ` Hans de Goede
2017-01-04 17:55               ` Jacob Pan
2017-01-07 22:23                 ` Jonathan Cameron
2017-01-08 10:15                   ` Hans de Goede
2017-01-08 10:34                     ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161214135525.16477-1-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --cc=russianneuromancer@ya.ru \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).