Hi Leela, > The current pmic i2c code assumes the current i2c bus is > the same as the pmic device's bus. There is nothing ensuring > that to be true. Therefore, select the proper bus before performing > a transaction. > > Signed-off-by: Aaron Durbin <adur...@chromium.org> > Signed-off-by: Simon Glass <s...@chromium.org> > Signed-off-by: Leela Krishna Amudala <l.kris...@samsung.com> > Reviewed-by: Doug Anderson <diand...@google.com> > --- > drivers/power/power_i2c.c | 62 > +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 > insertions(+), 5 deletions(-) > > diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c > index 47c606f..c22e01f 100644 > --- a/drivers/power/power_i2c.c > +++ b/drivers/power/power_i2c.c > @@ -16,9 +16,45 @@ > #include <i2c.h> > #include <compiler.h> > > +static int pmic_select(struct pmic *p) > +{ > + int ret, old_bus; > + > + old_bus = i2c_get_bus_num(); > + if (old_bus != p->bus) { > + debug("%s: Select bus %d\n", __func__, p->bus); > + ret = i2c_set_bus_num(p->bus); > + if (ret) { > + debug("%s: Cannot select pmic %s, err %d\n", > + __func__, p->name, ret); > + return -1; > + } > + } > + > + return old_bus; > +} > + > +static int pmic_deselect(int old_bus) > +{ > + int ret; > + > + if (old_bus != i2c_get_bus_num()) { > + ret = i2c_set_bus_num(old_bus); > + debug("%s: Select bus %d\n", __func__, old_bus); > + if (ret) { > + debug("%s: Cannot restore i2c bus, err %d\n", > + __func__, ret); > + return -1; > + } > + } > + > + return 0; > +} > + > int pmic_reg_write(struct pmic *p, u32 reg, u32 val) > { > unsigned char buf[4] = { 0 }; > + int ret, old_bus; > > if (check_reg(p, reg)) > return -1; > @@ -52,23 +88,33 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 > val) return -1; > } > > - if (i2c_write(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) > + old_bus = pmic_select(p); > + if (old_bus < 0) > return -1; > > - return 0; > + ret = i2c_write(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num);
I'm wondering if setting the bus before each, single i2c write (when we usually perform several writes to one device) will not be an overkill (we search the ll_entry_count linker list each time to find max i2c adapter names) ? The i2c_set_bus_num() is now done at pmic_probe(), but this also introduces overkill for "probing" each device when we want to read from it. As a side note - I would appreciate if you would add Stefano Babic and me on the Cc (as we are listed at e.g. power_core.c). > + pmic_deselect(old_bus); > + return ret; > } > > int pmic_reg_read(struct pmic *p, u32 reg, u32 *val) > { > unsigned char buf[4] = { 0 }; > u32 ret_val = 0; > + int ret, old_bus; > > if (check_reg(p, reg)) > return -1; > > - if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) > + old_bus = pmic_select(p); > + if (old_bus < 0) > return -1; > > + ret = i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num); > + pmic_deselect(old_bus); > + if (ret) > + return ret; > + > switch (pmic_i2c_tx_num) { > case 3: > if (p->sensor_byte_order == > PMIC_SENSOR_BYTE_ORDER_BIG) @@ -117,9 +163,15 @@ int > pmic_reg_update(struct pmic *p, int reg, uint regval) > int pmic_probe(struct pmic *p) > { > - i2c_set_bus_num(p->bus); > + int ret, old_bus; > + > + old_bus = pmic_select(p); > + if (old_bus < 0) > + return -1; > debug("Bus: %d PMIC:%s probed!\n", p->bus, p->name); > - if (i2c_probe(pmic_i2c_addr)) { > + ret = i2c_probe(pmic_i2c_addr); > + pmic_deselect(old_bus); > + if (ret) { > printf("Can't find PMIC:%s\n", p->name); > return -1; > } -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot