[U-Boot] [PATCH] fuelgauge: max17042: fix i2c read issue which causes infinity loop.
Minkyu Kang
mk7.kang at samsung.com
Wed Dec 11 03:27:54 CET 2013
Dear Przemyslaw Marczak,
On 11/12/13 00:19, Przemyslaw Marczak wrote:
> Issues:
> - reading i2c data by passing u16 pointer causes errors in read data.
> - max17042 status register fields have not only Power On Reset meaning
> so using proper mask is required.
>
> Changes:
> - read i2c data to type u32 instead of u16 - avoids buffer overflow
> - compare FG status register using mask not just one bit value
>
> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
> Cc: Lukasz Majewski <l.majewski at samsung.com>
> ---
> drivers/power/fuel_gauge/fg_max17042.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/fuel_gauge/fg_max17042.c b/drivers/power/fuel_gauge/fg_max17042.c
> index c285747..2cbf8cf 100644
> --- a/drivers/power/fuel_gauge/fg_max17042.c
> +++ b/drivers/power/fuel_gauge/fg_max17042.c
> @@ -28,11 +28,14 @@ static int fg_write_regs(struct pmic *p, u8 addr, u16 *data, int num)
>
> static int fg_read_regs(struct pmic *p, u8 addr, u16 *data, int num)
> {
> + unsigned int dat = 0;
initial value is unnecessary.
> int ret = 0;
> int i;
>
> - for (i = 0; i < num; i++, addr++)
> - ret |= pmic_reg_read(p, addr, (u32 *) (data + i));
> + for (i = 0; i < num; i++, addr++) {
> + ret |= pmic_reg_read(p, addr, &dat);
I think, need check return value.
if failed to read then you should not update data.
and such a case, do we need to read end of num?
why don't you return error directly?
I think this code should be..
ret = pmic_reg_read(p, addr, &dat);
if (ret)
return ret;
*(data + i) = (u16)dat;
> + *(data + i) = (u16) dat;
please remove space between ) and dat.
> + }
>
> return ret;
then it can be return 0; and initial value ( = 0) is unnecessary.
> }
> @@ -178,7 +181,7 @@ static int power_check_battery(struct pmic *p, struct pmic *bat)
> ret |= pmic_reg_read(p, MAX17042_STATUS, &val);
> debug("fg status: 0x%x\n", val);
>
> - if (val == MAX17042_POR)
> + if (val && MAX17042_POR)
if (val & MAX17042_POR) ?
> por_fuelgauge_init(p);
>
> ret |= pmic_reg_read(p, MAX17042_VERSION, &val);
>
Thanks,
Minkyu Kang.
More information about the U-Boot
mailing list