From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 23 Mar 2016 07:16:27 +0100
Subject: [U-Boot] [PATCH 2/6 v2] i2c: designware_i2c: Add
 dw_i2c_enable() helper function
In-Reply-To: <1458711988-3235-2-git-send-email-sr@denx.de>
References: <1458711988-3235-1-git-send-email-sr@denx.de>
 <1458711988-3235-2-git-send-email-sr@denx.de>
Message-ID: <56F234BB.8010709@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Heiko Schocher <hs@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