scottk/OpenCellular
1
0
Fork 0
mirror of https://github.com/Telecominfraproject/OpenCellular.git synced 2026-01-11 18:35:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

charger/isl923x: Protect CONTROL1 read-modify-write with a mutex

Browse Source 
CONTROL1 bits can be modified from multiple tasks:
 - charger_enable_otg_power (charger or pd task)
 - charger_discharge_on_ac (host command or console)
 - charger_enable/disable_psys (chipset task)
 - print_amon_bmon (console)

Since we use I2C read, modify, then I2C write access pattern,
there is a small chance of races between these accesses:
let's protect them with a mutex.

Also, the current code sometimes uses charger_get_option/set_option
instead of manipulating CONTROL0 directly: fix those to regain
a bit of the extra code size caused by the mutex.

BRANCH=none
BUG=b:67029560
TEST=Flash lux, battery charges, amon works fine.
TEST=Flash elm, battery charges.

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

Change-Id: If375d9922db53dd582582bfa44d6218fe0b416ec
Reviewed-on: https://chromium-review.googlesource.com/848486
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>
This commit is contained in:
Nicolas Boichat
2018-01-03 10:47:56 +08:00
committed by chrome-bot
parent 51b6222520
commit f14879eae1
1 changed files with 48 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
driver/charger/isl923x.c
View File

@@ -14,6 +14,7 @@
#include "hooks.h"
#include "i2c.h"
#include "isl923x.h"
#include "task.h"
#include "timer.h"
#include "util.h"
@@ -31,6 +32,9 @@
static int learn_mode;
/* Mutex for CONTROL1 register, that can be updated from multiple tasks. */
static struct mutex control1_mutex;
/* Charger parameters */
static const struct charger_info isl9237_charger_info = {
.name = CHARGER_NAME,
@@ -102,16 +106,23 @@ int charger_enable_otg_power(int enabled)
{
int rv, control1;
mutex_lock(&control1_mutex);
rv = raw_read16(ISL923X_REG_CONTROL1, &control1);
if (rv)
return rv;
goto out;
if (enabled)
control1 |= ISL923X_C1_OTG;
else
control1 &= ~ISL923X_C1_OTG;
return raw_write16(ISL923X_REG_CONTROL1, control1);
rv = raw_write16(ISL923X_REG_CONTROL1, control1);
out:
mutex_unlock(&control1_mutex);
return rv;
}
/*
@@ -308,14 +319,14 @@ static void isl923x_init(void)
#ifdef CONFIG_CHARGE_RAMP_HW
#ifdef CONFIG_CHARGER_ISL9237
if (charger_get_option(&reg))
if (raw_read16(ISL923X_REG_CONTROL0, &reg))
goto init_fail;
/* Set input voltage regulation reference voltage for charge ramp */
reg &= ~ISL9237_C0_VREG_REF_MASK;
reg |= ISL9237_C0_VREG_REF_4200;
if (charger_set_option(reg))
if (raw_write16(ISL923X_REG_CONTROL0, reg))
goto init_fail;
#else /* !defined(CONFIG_CHARGER_ISL9237) */
/*
@@ -329,13 +340,13 @@ static void isl923x_init(void)
goto init_fail;
#endif /* defined(CONFIG_CHARGER_ISL9237) */
#else /* !defined(CONFIG_CHARGE_RAMP_HW) */
if (charger_get_option(&reg))
if (raw_read16(ISL923X_REG_CONTROL0, &reg))
goto init_fail;
/* Disable voltage regulation loop to disable charge ramp */
reg |= ISL923X_C0_DISABLE_VREG;
if (charger_set_option(reg))
if (raw_write16(ISL923X_REG_CONTROL0, reg))
goto init_fail;
#endif /* defined(CONFIG_CHARGE_RAMP_HW) */
@@ -368,9 +379,11 @@ int charger_discharge_on_ac(int enable)
int rv;
int control1;
mutex_lock(&control1_mutex);
rv = raw_read16(ISL923X_REG_CONTROL1, &control1);
if (rv)
return rv;
goto out;
control1 &= ~ISL923X_C1_LEARN_MODE_AUTOEXIT;
if (enable)
@@ -381,6 +394,9 @@ int charger_discharge_on_ac(int enable)
rv = raw_write16(ISL923X_REG_CONTROL1, control1);
learn_mode = !rv && enable;
out:
mutex_unlock(&control1_mutex);
return rv;
}
@@ -392,7 +408,7 @@ int charger_set_hw_ramp(int enable)
{
int rv, reg;
rv = charger_get_option(&reg);
rv = raw_read16(ISL923X_REG_CONTROL0, &reg);
if (rv)
return rv;
@@ -402,7 +418,7 @@ int charger_set_hw_ramp(int enable)
else
reg |= ISL923X_C0_DISABLE_VREG;
return charger_set_option(reg);
return raw_write16(ISL923X_REG_CONTROL0, reg);
}
int chg_ramp_is_stable(void)
@@ -440,6 +456,8 @@ static void charger_enable_psys(void)
{
int val;
mutex_lock(&control1_mutex);
/*
* enable system power monitor PSYS function
*/
@@ -447,6 +465,8 @@ static void charger_enable_psys(void)
val |= ISL923X_C1_ENABLE_PSYS;
raw_write16(ISL923X_REG_CONTROL1, val);
}
mutex_unlock(&control1_mutex);
}
DECLARE_HOOK(HOOK_CHIPSET_STARTUP, charger_enable_psys, HOOK_PRIO_DEFAULT);
@@ -454,6 +474,8 @@ static void charger_disable_psys(void)
{
int val;
mutex_lock(&control1_mutex);
/*
* disable system power monitor PSYS function
*/
@@ -461,6 +483,8 @@ static void charger_disable_psys(void)
val &= ~ISL923X_C1_ENABLE_PSYS;
raw_write16(ISL923X_REG_CONTROL1, val);
}
mutex_unlock(&control1_mutex);
}
DECLARE_HOOK(HOOK_CHIPSET_SHUTDOWN, charger_disable_psys, HOOK_PRIO_DEFAULT);
@@ -537,21 +561,25 @@ static int print_amon_bmon(enum amon_bmon amon, int direction,
return ret;
#endif
mutex_lock(&control1_mutex);
ret = i2c_read16(I2C_PORT_CHARGER, I2C_ADDR_CHARGER,
ISL923X_REG_CONTROL1, &reg);
if (ret)
return ret;
if (!ret) {
/* Switch between AMON/BMON */
if (amon == AMON)
reg &= ~ISL923X_C1_SELECT_BMON;
else
reg |= ISL923X_C1_SELECT_BMON;
/* Switch between AMON/BMON */
if (amon == AMON)
reg &= ~ISL923X_C1_SELECT_BMON;
else
reg |= ISL923X_C1_SELECT_BMON;
/* Enable monitor */
reg &= ~ISL923X_C1_DISABLE_MON;
ret = i2c_write16(I2C_PORT_CHARGER, I2C_ADDR_CHARGER,
ISL923X_REG_CONTROL1, reg);
}
mutex_unlock(&control1_mutex);
/* Enable monitor */
reg &= ~ISL923X_C1_DISABLE_MON;
ret = i2c_write16(I2C_PORT_CHARGER, I2C_ADDR_CHARGER,
ISL923X_REG_CONTROL1, reg);
if (ret)
return ret;