On 05/Mar/2019 19:32, Marek Vasut wrote: > On 3/5/19 12:23 PM, Ismael Luceno Cortes wrote: > > Cosmetic change. Any call to the recover function would need to do the > > same check afterwards, so it's sensible to make it part of the function. > > > > Signed-off-by: Ismael Luceno <ismael.luc...@silicon-gears.com> > > --- > > > > Notes: > > Changes since v1: > > - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC > > support") > > - Explained the change > > - Replaced C99-style variable declaration > > > > drivers/i2c/rcar_i2c.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c > > index 4556b115bd..21bcf09101 100644 > > --- a/drivers/i2c/rcar_i2c.c > > +++ b/drivers/i2c/rcar_i2c.c > > @@ -77,12 +77,13 @@ static int rcar_i2c_finish(struct udevice *dev) > > return ret; > > } > > > > -static void rcar_i2c_recover(struct udevice *dev) > > +static int rcar_i2c_recover(struct udevice *dev) > > { > > struct rcar_i2c_priv *priv = dev_get_priv(dev); > > u32 mcr = RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_OBPC; > > u32 mcra = mcr | RCAR_I2C_ICMCR_FSDA; > > int i; > > + u32 ret; > > Can you change this to "reg" or something like that ? $ret is usually a > signed int return value, so this could be confusing.
Ok. > > > > /* Send 9 SCL pulses */ > > for (i = 0; i < 9; i++) { > > @@ -102,6 +103,9 @@ static void rcar_i2c_recover(struct udevice *dev) > > udelay(5); > > writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR); > > udelay(5); > > + > > + ret = readl(priv->base + RCAR_I2C_ICMSR); > > + return ret & RCAR_I2C_ICMCR_FSDA; > > What about doing this instead > > reg = readl... > if (reg & ...FSDA) > return -EBUSY; > > return 0; > > That way, the code will use standard errno.h return values. Makes sense. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot