mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: rklein@nvidia.com
To: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: olof@lixom.net, linux-kernel@vger.kernel.org,
	Rhyland Klein <rklein@nvidia.com>
Subject: [PATCH] power: bq20z75: fix issues with present and suspend
Date: Wed,  9 Mar 2011 16:18:02 -0800	[thread overview]
Message-ID: <1299716282-21857-1-git-send-email-rklein@nvidia.com> (raw)

From: Rhyland Klein <rklein@nvidia.com>

There are a few issues found around the battery not being present. If the
battery isn't present, then a few undesirable things happen. The first was
excessive reporting of failed properties. This was fixed by instead returning
ENODATA for all properties other than PRESENT if the battery isn't present.
That way the callers can identify the difference between a failure and the
battery not being there.

The next issue was in the suspend logic. It was found that if the battery wasn't
present, then it would return a failure, preventing the system from going into
suspend. If there is no battery present, the io is expected to fail, so in that
case, we shouldn't return the failure and just acknowledge that it was expected.

I also found that when a gpio was used, i didn't maintain the internal
is_present state properly. I added a set of that to fix that.

Lastly, the code to see io's fail and figure out that the battery isn't present
when not using a gpio had a problem. In that code, it looked for the read to
fail and if it did, then handled it. The problem is that in function to get the
property, it first writes a value and that write can fail, causing the code
to never reach the logic after the read. Fix is to move the logic till after
the write.

Change-Id: I09c9a9f0c1f0bcbad2308b6ffc65f6823d6a7ad0
Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/power/bq20z75.c |   32 +++++++++++++++++++++-----------
 1 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
index 8018e8c..5615c92 100644
--- a/drivers/power/bq20z75.c
+++ b/drivers/power/bq20z75.c
@@ -171,7 +171,7 @@ static int bq20z75_read_word_data(struct i2c_client *client, u8 address)
 	}
 
 	if (ret < 0) {
-		dev_warn(&client->dev,
+		dev_dbg(&client->dev,
 			"%s: i2c read at address 0x%x failed\n",
 			__func__, address);
 		return ret;
@@ -199,7 +199,7 @@ static int bq20z75_write_word_data(struct i2c_client *client, u8 address,
 	}
 
 	if (ret < 0) {
-		dev_warn(&client->dev,
+		dev_dbg(&client->dev,
 			"%s: i2c write to address 0x%x failed\n",
 			__func__, address);
 		return ret;
@@ -223,6 +223,7 @@ static int bq20z75_get_battery_presence_and_health(
 			val->intval = 1;
 		else
 			val->intval = 0;
+		bq20z75_device->is_present = val->intval;
 		return ret;
 	}
 
@@ -232,18 +233,17 @@ static int bq20z75_get_battery_presence_and_health(
 	ret = bq20z75_write_word_data(client,
 		bq20z75_data[REG_MANUFACTURER_DATA].addr,
 		MANUFACTURER_ACCESS_STATUS);
-	if (ret < 0)
-		return ret;
-
-
-	ret = bq20z75_read_word_data(client,
-		bq20z75_data[REG_MANUFACTURER_DATA].addr);
 	if (ret < 0) {
 		if (psp == POWER_SUPPLY_PROP_PRESENT)
 			val->intval = 0; /* battery removed */
 		return ret;
 	}
 
+	ret = bq20z75_read_word_data(client,
+		bq20z75_data[REG_MANUFACTURER_DATA].addr);
+	if (ret < 0)
+		return ret;
+
 	if (ret < bq20z75_data[REG_MANUFACTURER_DATA].min_value ||
 	    ret > bq20z75_data[REG_MANUFACTURER_DATA].max_value) {
 		val->intval = 0;
@@ -455,6 +455,8 @@ static int bq20z75_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_PRESENT:
 	case POWER_SUPPLY_PROP_HEALTH:
 		ret = bq20z75_get_battery_presence_and_health(client, psp, val);
+		if (psp == POWER_SUPPLY_PROP_PRESENT)
+			return 0;
 		break;
 
 	case POWER_SUPPLY_PROP_TECHNOLOGY:
@@ -516,9 +518,16 @@ done:
 	}
 
 	dev_dbg(&client->dev,
-		"%s: property = %d, value = %d\n", __func__, psp, val->intval);
+		"%s: property = %d, value = %x\n", __func__, psp, val->intval);
+
+	if (ret && bq20z75_device->is_present)
+		return ret;
+
+	/* battery not present, so return NODATA for properties */
+	if (ret)
+		return -ENODATA;
 
-	return ret;
+	return 0;
 }
 
 static irqreturn_t bq20z75_irq(int irq, void *devid)
@@ -643,13 +652,14 @@ static int __devexit bq20z75_remove(struct i2c_client *client)
 static int bq20z75_suspend(struct i2c_client *client,
 	pm_message_t state)
 {
+	struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
 	s32 ret;
 
 	/* write to manufacturer access with sleep command */
 	ret = bq20z75_write_word_data(client,
 		bq20z75_data[REG_MANUFACTURER_DATA].addr,
 		MANUFACTURER_ACCESS_SLEEP);
-	if (ret < 0)
+	if (bq20z75_device->is_present && ret < 0)
 		return ret;
 
 	return 0;
-- 
1.7.0.4


             reply	other threads:[~2011-03-10  0:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-10  0:18 rklein [this message]
2011-03-16 13:48 ` Anton Vorontsov

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=1299716282-21857-1-git-send-email-rklein@nvidia.com \
    --to=rklein@nvidia.com \
    --cc=cbouatmailru@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olof@lixom.net \
    /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

Powered by JetHome