[U-Boot] [PATCH v2 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover

[Ismael Luceno Cortes]

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.luceno at 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.