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

[Marek Vasut]

On 3/5/19 11:03 AM, Ismael Luceno Cortes wrote:
> On 04/Mar/2019 20:47, Marek Vasut wrote:
>> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
>>
>> Why is this change needed ? A commit message explaining it would help a lot.
> 
> It's cosmetic. Any call to the recover function would need to do the
> same check afterwards, so it' sensible to make it part of the function.

This needs to be in the commit message then.

>>> Signed-off-by: Ismael Luceno <ismael.luceno at silicon-gears.com>
>>> ---
>>>  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 5e04b68d95..b829e8b20d 100644
>>> --- a/drivers/i2c/rcar_i2c.c
>>> +++ b/drivers/i2c/rcar_i2c.c
>>> @@ -68,7 +68,7 @@ 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;
>>> @@ -93,6 +93,9 @@ static void rcar_i2c_recover(struct udevice *dev)
>>>  	udelay(5);
>>>  	writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
>>>  	udelay(5);
>>> +
>>> +	u32 val = readl(priv->base + RCAR_I2C_ICMSR);
>>> +	return val & RCAR_I2C_ICMCR_FSDA;
>>
>> Declare all your variables at the beginning of the function, I believe
>> the compiler would even complain about this.
> 
> The default mode is -std=gnu11 since GCC 5, and the Makefile defines
> CSTD_FLAG to that too.
> 
>>>  }
>>>  
>>>  static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
>>> @@ -108,12 +111,11 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
>>>  	writel(0, priv->base + RCAR_I2C_ICMSR);
>>>  	writel(priv->icccr, priv->base + RCAR_I2C_ICCCR);
>>>  
>>> +	/* Wait for the bus */
>>>  	ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMCR,
>>>  				RCAR_I2C_ICMCR_FSDA, false, 2, true);
>>>  	if (ret) {
>>> -		rcar_i2c_recover(dev);
>>> -		val = readl(priv->base + RCAR_I2C_ICMSR);
>>> -		if (val & RCAR_I2C_ICMCR_FSDA) {
>>> +		if (rcar_i2c_recover(dev)) {
>>>  			dev_err(dev, "Bus busy, aborting\n");
>>>  			return ret;
>>>  		}


-- 
Best regards,
Marek Vasut