Fwd: [PATCH 1/3] i2c: Add Device Tree support to the Nomadik I2C driver

Gah, what a thread...

On Fri, Jun 15, 2012 at 3:18 PM, Lee Jones [off-list ref] wrote:

On 15/06/12 14:05, Srinidhi Kasagar wrote:

quoted

On Fri, Jun 15, 2012 at 14:45:11 +0200, Lee Jones wrote:

quoted

On 15/06/12 12:50, Srinidhi Kasagar wrote:

quoted

quoted

quoted

quoted

+static struct nmk_i2c_controller u8500_i2c = {
+ ? ? ? /*
+ ? ? ? ?* Slave data setup time; 250ns, 100ns, and 10ns, which
+ ? ? ? ?* is 14, 6 and 2 respectively for a 48Mhz i2c clock.
+ ? ? ? ?*/
+ ? ? ? .slsu ? ? ? ? ? = 0xe,
+ ? ? ? .tft ? ? ? ? ? ?= 1, ? ? ?/* Tx FIFO threshold */
+ ? ? ? .rft ? ? ? ? ? ?= 8, ? ? ?/* Rx FIFO threshold */
+ ? ? ? .clk_freq ? ? ? = 100000, /* std. mode operation */
+ ? ? ? .timeout ? ? ? ?= 200, ? ?/* Slave response timeout(ms) */
+ ? ? ? .sm ? ? ? ? ? ? = I2C_FREQ_MODE_FAST,

How is this possible? you are setting clk_freq as 100kb/s and mode
as fast mode which is supposed to be 400kb/s.

That's not how I read it:

But it is not readable. It confuses people.

I understood it. :)

If you think it's unclear speak to the author, Linus. He's CC'ed.

Author of what? The i2c driver was written by Srinidhi. (The
MODULE_AUTHOR() clause should be a good hint...)

But it's true that board data for the ux500 kernel use
100000 Hz and I2C_FREQ_MODE_FAST, in the board-mop500.c
file, like this:

U8500_I2C_CONTROLLER(0, 0xe, 1, 8, 100000, 200, I2C_FREQ_MODE_FAST);
U8500_I2C_CONTROLLER(1, 0xe, 1, 8, 100000, 200, I2C_FREQ_MODE_FAST);
U8500_I2C_CONTROLLER(2, 0xe, 1, 8, 100000, 200, I2C_FREQ_MODE_FAST);
U8500_I2C_CONTROLLER(3, 0xe, 1, 8, 100000, 200, I2C_FREQ_MODE_FAST);

This is indeed a bit odd. But obviously it works... But reading some
fixes in another tree it seems it should look like this:

/*
 * The board uses 4 i2c controllers, initialize all of
 * them with slave data setup time of 250 ns,
 * Tx & Rx FIFO threshold values as 1 and standard
 * mode of operation
 */
U8500_I2C_CONTROLLER(0, 0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
U8500_I2C_CONTROLLER(1, 0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
U8500_I2C_CONTROLLER(2, 0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
U8500_I2C_CONTROLLER(3, 0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);

Which makes *much* more sense.

I'll cook a separate patch for this or something.

Thanks,
Linus Walleij