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

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;

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;

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
participants (3)
-
Hans de Goede
-
Heiko Schocher
-
Stefan Roese