[U-Boot] [PATCH 2/6 v2] i2c: designware_i2c: Add dw_i2c_enable() helper function

[Marek Vasut]

On 03/23/2016 06:46 AM, Stefan Roese wrote:
> dw_i2c_enable() is used to dis-/en-able the I2C controller. It makes
> sense to add such a function, as the controller is dis-/en-abled
> multiple times in the code. Additionally, this function now checks,
> if the controller is really dis-/en-abled. This code is copied
> from the Linux I2C driver version.
> 
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Heiko Schocher <hs at denx.de>
> ---
> v2:
> - Use true / false for binary values as suggested by Bin
> 
>  drivers/i2c/designware_i2c.c | 46 +++++++++++++++++++++++++-------------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c
> index e768cde..2b0c8cb 100644
> --- a/drivers/i2c/designware_i2c.c
> +++ b/drivers/i2c/designware_i2c.c
> @@ -34,6 +34,26 @@ static struct i2c_regs *i2c_get_base(struct i2c_adapter *adap)
>  	return NULL;
>  }
>  
> +static void dw_i2c_enable(struct i2c_regs *i2c_base, bool enable)
> +{
> +	int timeout = 100;
> +
> +	do {
> +		writel(enable, &i2c_base->ic_enable);

I believe boolean true in C can be anything non-zero, not necessarily
one (1). It is safer to be explicit and do
  enable = enable ? IC_ENABLE_0B : 0;
at the beginning of the function

> +		if ((readl(&i2c_base->ic_enable_status) & 1) == enable)

Also drop this & 1 and make it IC_ENABLE_0B please.

> +			return;
> +
> +		/*
> +		 * Wait 10 times the signaling period of the highest I2C
> +		 * transfer supported by the driver (for 400KHz this is
> +		 * 25us) as described in the DesignWare I2C databook.
> +		 */
> +		udelay(25);
> +	} while (timeout--);

It's a shame we cannot flip this to wait_for_bit() :(

> +	printf("timeout in %sabling I2C adapter\n", enable ? "en" : "dis");
> +}
[..]
Best regards,
Marek Vasut