| Message ID | 1364861055-21670-4-git-send-email-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
Hi Simon, Just had one following comment On Tue, Apr 2, 2013 at 5:34 AM, Simon Glass <sjg@chromium.org> wrote: > From: Aaron Durbin <adurbin@chromium.org> > > 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 <adurbin@chromium.org> > Signed-off-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > drivers/power/power_i2c.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c > index 3e5a784..ec9701e 100644 > --- a/drivers/power/power_i2c.c > +++ b/drivers/power/power_i2c.c > @@ -39,6 +39,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) > if (check_reg(p, reg)) > return -1; > > + I2C_SET_BUS(p->bus); > + Do we need to set I2C bus for each register read and write? > switch (pmic_i2c_tx_num) { > case 3: > if (p->sensor_byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) { > @@ -82,6 +84,8 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val) > if (check_reg(p, reg)) > return -1; > > + I2C_SET_BUS(p->bus); > + > if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) > return -1; > > -- > 1.8.1.3 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Rajeshwari, On Mon, Apr 1, 2013 at 11:18 PM, Rajeshwari Birje <rajeshwari.birje@gmail.com> wrote: > Hi Simon, > > Just had one following comment > > On Tue, Apr 2, 2013 at 5:34 AM, Simon Glass <sjg@chromium.org> wrote: >> From: Aaron Durbin <adurbin@chromium.org> >> >> 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 <adurbin@chromium.org> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> --- >> drivers/power/power_i2c.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c >> index 3e5a784..ec9701e 100644 >> --- a/drivers/power/power_i2c.c >> +++ b/drivers/power/power_i2c.c >> @@ -39,6 +39,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) >> if (check_reg(p, reg)) >> return -1; >> >> + I2C_SET_BUS(p->bus); >> + > > Do we need to set I2C bus for each register read and write? It could potentially be done at a higher level, but it is tricky, since the pmic is supposed to know its bus number. Nothing else should need to know. It's just setting a variable so does not take long. I think this sort of thing will get cleaned up now that Heiko has posted the i2c clean-up series (which I am going to take a look at). Regards, Simon >> switch (pmic_i2c_tx_num) { >> case 3: >> if (p->sensor_byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) { >> @@ -82,6 +84,8 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val) >> if (check_reg(p, reg)) >> return -1; >> >> + I2C_SET_BUS(p->bus); >> + >> if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) >> return -1; >> >> -- >> 1.8.1.3 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot > -- > Regards, > Rajeshwari Shinde
diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index 3e5a784..ec9701e 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -39,6 +39,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p->bus); + switch (pmic_i2c_tx_num) { case 3: if (p->sensor_byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) { @@ -82,6 +84,8 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p->bus); + if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) return -1;