diff mbox

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

Message ID 1426667454-9344-1-git-send-email-sr@denx.de
State Awaiting Upstream
Delegated to: Heiko Schocher
Headers show

Commit Message

Stefan Roese March 18, 2015, 8:30 a.m. UTC
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(-)

Comments

Hans de Goede March 18, 2015, 8:41 a.m. UTC | #1
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;
>
Heiko Schocher March 18, 2015, 9:14 a.m. UTC | #2
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
diff mbox

Patch

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;