[U-Boot] [PATCH] mx53loco: Remove unneeded 'retval' variable

[Wolfgang Denk]

Dear Fabio Estevam,

In message <1356604017-9699-1-git-send-email-festevam at gmail.com> you wrote:
> From: Fabio Estevam <fabio.estevam at freescale.com>
> 
> commit c73368150 (pmic: Extend PMIC framework to support multiple instances 
> of PMIC devices) introduced an extra 'retval' variable, but this is not 
> necessary since we have already the variable 'ret' in place.
> 
> So use 'ret' to store the return values from the pmic related calls and remove
> 'retval'.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>

Hm...

I think the hole error code handling is borked in this function.

Assume you enter this branch:

349
350         if (!i2c_probe(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR)) {

This will set "ret":

361                 ret = pmic_reg_write(p, DA9053_BUCKCORE_REG, val);
362
363                 ret |= pmic_reg_read(p, DA9053_SUPPLY_REG, &val);
...
365                 ret |= pmic_reg_write(p, DA9053_SUPPLY_REG, val);
...
368                 ret |= pmic_reg_write(p, DA9053_BUCKPRO_REG, 0x62);
369                 ret |= pmic_reg_write(p, DA9053_SUPPLY_REG, 0x62);
370         }

Assume any of these calls returns an error condition.

Now we enter the second branch:

371
372         if (!i2c_probe(CONFIG_SYS_FSL_PMIC_I2C_ADDR)) {
...

But here we will unconditionally set "ret", no matter what it
contained before:

384                 ret = pmic_reg_write(p, REG_SW_0, val);


So if both code sections for DIALOG_PMIC_I2C_ADDR and for
FSL_PMIC_I2C_ADDR get executed, then any errors in the first part go
undetected...



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
An Elephant is a mouse with an Operating System.              - Knuth