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

Marek Vasut
2016-03-23T06:16:27Z

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