Hi,
On 17-03-15 11:08, 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.
Hmm, this does not match with what the Allwinner datasheets say: https://github.com/allwinner-zh/documents/blob/master/A20/A20%20user%20manua...
They say:
Fsamp = F 0 = Fin / 2^CLK_N F1 = F0 / (CLK_M + 1) Foscl = F1 / 10 = Fin / (2^CLK_N * (CLK_M + 1)*10)
With Foscl being the ultimate i2c speed. Notice that they are talking about 2 ^ CLK_N not 2 ^ (CLK_N + 1)
And they have a few examples which match this. Now it could be that a register value of 0 means CLK_N = 1, reg 1 CLK_N = 2, etc. this is not clearly specified ...
This new function is taken from the Linux kernel.
Interesting, because on Allwinnner / sunxi devices we are using the kernel driver formula unmodified, and things seem to work fine despite this. Could be tolerances allowing this, could be the Allwinner datasheet being unclear.
I've send allwinner a mail asking them to clarify this.
This was detected and tested on the Marvell Armada A38x DB-88F6820-GP eval board.
So I take it you connected a memory oscilloscope to the i2c wires ?
Regards,
Hans
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
drivers/i2c/mvtwsi.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 9b2ca1e..320235c 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -228,13 +228,10 @@ 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) +{
- return CONFIG_SYS_TCLK / (10 * (m + 1) * (2 << n));
+}
/*
- Reset controller.
@@ -266,7 +263,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;