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@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.