[PATCH v3] power: supply: bq27xxx: Retrieve again when busy

[Jerry Lv]

Multiple applications may access the battery gauge at the same time, so
the gauge may be busy and EBUSY will be returned. The driver will set a
flag to record the EBUSY state, and this flag will be kept until the next
periodic update. When this flag is set, bq27xxx_battery_get_property()
will just return ENODEV until the flag is updated.

Even if the gauge was busy during the last accessing attempt, returning
ENODEV is not ideal, and can cause confusion in the applications layer.

Instead, retry accessing the I2C to update the flag is as expected, for
the gauge typically recovers from busy state within a few milliseconds.
If still failed to access the gauge, the real error code would be returned
instead of ENODEV (as suggested by Pali Rohár).

Signed-off-by: Jerry Lv <Jerry.Lv@xxxxxxxx>
---
Changes in v3:
- Move I2C retry logic to bq27xxx_battery_i2c_read()
- Link to v2: https://lore.kernel.org/all/20241029-foo-fix-v1-1-1dbfed72d023@xxxxxxxx

Changes in v2:
- Retry up to 3 times when gauge is busy
- return the real error code when fail to access the device
- Link to v1: https://lore.kernel.org/all/20240913-foo-fix2-v1-1-a0f499404f3a@xxxxxxxx
---
 drivers/power/supply/bq27xxx_battery.c     |  2 +-
 drivers/power/supply/bq27xxx_battery_i2c.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 750fda543308..99631ab46e8e 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -2030,7 +2030,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
 	mutex_unlock(&di->lock);
 
 	if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
-		return -ENODEV;
+		return di->cache.flags;
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index ba0d22d90429..78cccea4d419 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -6,6 +6,7 @@
  *	Andrew F. Davis <afd@xxxxxx>
  */
 
+#include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
@@ -27,10 +28,15 @@ static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
 static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
 				    bool single)
 {
+#define MAX_RETRY 3
+#define I2C_DELAY_MIN 10000
+#define I2C_DELAY_MAX 11000
+
 	struct i2c_client *client = to_i2c_client(di->dev);
 	struct i2c_msg msg[2];
 	u8 data[2];
 	int ret;
+	int retry = 0;
 
 	if (!client->adapter)
 		return -ENODEV;
@@ -47,7 +53,14 @@ static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
 	else
 		msg[1].len = 2;
 
-	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+	do {
+		ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+		if (ret == -EBUSY && retry < MAX_RETRY) {
+			retry++;
+			/* sleep 10 miliseconds when busy */
+			usleep_range(I2C_DELAY_MIN, I2C_DELAY_MAX);
+		}
+	} while (ret == -EBUSY && retry < MAX_RETRY);
 	if (ret < 0)
 		return ret;
 

---
base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
change-id: 20241008-foo-fix-b2244cbe6dce

Best regards,
-- 
Jerry Lv <Jerry.Lv@xxxxxxxx>