[U-Boot] [PATCH v2] i2c: mvtwsi: Fix problem with baud rate calculation

[U-Boot] [PATCH v2] i2c: mvtwsi: Fix problem with baud rate calculation

[Stefan Roese]

The current implementation for baudrate calculation is incorrect.
This part from the formula:

"2 ^ (n + 1)" is not equivalent to (1 << n) but to (2 << n)!

This patch fixes this and moves this calculation to a function instead of using a macro.
This new function is taken from the Linux kernel.

This was detected and tested on the Marvell Armada A38x DB-88F6820-GP eval board.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Prafulla Wadaskar <prafulla@marvell.com>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Ian Campbell <ijc@hellion.org.uk>
Cc: Heiko Schocher <hs@denx.de>
---
v2:
- Still use the old formula for SUNXI, as this was checked
  by Hans with Allwinner to be the correct formula. This means
  that the Linux baudrate is incorrect for SUNXI I2C though.

 drivers/i2c/mvtwsi.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 9b2ca1e..6f6edd5 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -228,13 +228,14 @@ static int twsi_stop(int status)
 	return status;
 }
 
-/*
- * Ugly formula to convert m and n values to a frequency comes from
- * TWSI specifications
- */
-
-#define TWSI_FREQUENCY(m, n) \
-	(CONFIG_SYS_TCLK / (10 * (m + 1) * (1 << n)))
+static unsigned int twsi_calc_freq(const int n, const int m)
+{
+#ifdef CONFIG_SUNXI
+	return CONFIG_SYS_TCLK / (10 * (m + 1) * (1 << n));
+#else
+	return CONFIG_SYS_TCLK / (10 * (m + 1) * (2 << n));
+#endif
+}
 
 /*
  * Reset controller.
@@ -266,7 +267,7 @@ static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 	/* compute m, n setting for highest speed not above requested speed */
 	for (n = 0; n < 8; n++) {
 		for (m = 0; m < 16; m++) {
-			tmp_speed = TWSI_FREQUENCY(m, n);
+			tmp_speed = twsi_calc_freq(n, m);
 			if ((tmp_speed <= requested_speed)
 			 && (tmp_speed > highest_speed)) {
 				highest_speed = tmp_speed;
-- 
2.3.3

[U-Boot] [PATCH v2] i2c: mvtwsi: Fix problem with baud rate calculation

[Hans de Goede]

Hi,

On 18-03-15 09:30, Stefan Roese wrote:
> The current implementation for baudrate calculation is incorrect.
> This part from the formula:
>
> "2 ^ (n + 1)" is not equivalent to (1 << n) but to (2 << n)!
>
> This patch fixes this and moves this calculation to a function instead of using a macro.
> This new function is taken from the Linux kernel.
>
> This was detected and tested on the Marvell Armada A38x DB-88F6820-GP eval board.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Prafulla Wadaskar <prafulla@marvell.com>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Ian Campbell <ijc@hellion.org.uk>
> Cc: Heiko Schocher <hs@denx.de>
> ---
> v2:
> - Still use the old formula for SUNXI, as this was checked
>    by Hans with Allwinner to be the correct formula. This means
>    that the Linux baudrate is incorrect for SUNXI I2C though.
>


Thanks for the v2. This looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

>   drivers/i2c/mvtwsi.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index 9b2ca1e..6f6edd5 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -228,13 +228,14 @@ static int twsi_stop(int status)
>   	return status;
>   }
>
> -/*
> - * Ugly formula to convert m and n values to a frequency comes from
> - * TWSI specifications
> - */
> -
> -#define TWSI_FREQUENCY(m, n) \
> -	(CONFIG_SYS_TCLK / (10 * (m + 1) * (1 << n)))
> +static unsigned int twsi_calc_freq(const int n, const int m)
> +{
> +#ifdef CONFIG_SUNXI
> +	return CONFIG_SYS_TCLK / (10 * (m + 1) * (1 << n));
> +#else
> +	return CONFIG_SYS_TCLK / (10 * (m + 1) * (2 << n));
> +#endif
> +}
>
>   /*
>    * Reset controller.
> @@ -266,7 +267,7 @@ static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
>   	/* compute m, n setting for highest speed not above requested speed */
>   	for (n = 0; n < 8; n++) {
>   		for (m = 0; m < 16; m++) {
> -			tmp_speed = TWSI_FREQUENCY(m, n);
> +			tmp_speed = twsi_calc_freq(n, m);
>   			if ((tmp_speed <= requested_speed)
>   			 && (tmp_speed > highest_speed)) {
>   				highest_speed = tmp_speed;
>

[U-Boot] [PATCH v2] i2c: mvtwsi: Fix problem with baud rate calculation

[Heiko Schocher]

Hello Stefan,

Am 18.03.2015 09:30, schrieb Stefan Roese:
> The current implementation for baudrate calculation is incorrect.
> This part from the formula:
>
> "2 ^ (n + 1)" is not equivalent to (1 << n) but to (2 << n)!
>
> This patch fixes this and moves this calculation to a function instead of using a macro.
> This new function is taken from the Linux kernel.
>
> This was detected and tested on the Marvell Armada A38x DB-88F6820-GP eval board.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Prafulla Wadaskar <prafulla@marvell.com>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Ian Campbell <ijc@hellion.org.uk>
> Cc: Heiko Schocher <hs@denx.de>
> ---
> v2:
> - Still use the old formula for SUNXI, as this was checked
>    by Hans with Allwinner to be the correct formula. This means
>    that the Linux baudrate is incorrect for SUNXI I2C though.
>
>   drivers/i2c/mvtwsi.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)

Thanks!

Applied to u-boot-i2c.git

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany