On 18/06/12 08:58, Srinidhi Kasagar wrote:
On Mon, Jun 18, 2012 at 09:18:05 +0200, Lee Jones wrote:
On 17/06/12 18:43, Linus Walleij wrote:
Gah, what a thread...
On Fri, Jun 15, 2012 at 3:18 PM, Lee Jones<lee.jones@xxxxxxxxxx> wrote:
On 15/06/12 14:05, Srinidhi Kasagar wrote:
On Fri, Jun 15, 2012 at 14:45:11 +0200, Lee Jones wrote:
On 15/06/12 12:50, Srinidhi Kasagar wrote:
+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...)
Actually, we appear to have our wires crossed. You wrote the board data
stuff below, which changes the mode from STANDARD to FAST, no doubt
using Srinidhi's struct explanation comments found in
arch/arm/plat-nomadik/include/plat/i2c.h.
> I'll cook a separate patch for this or something.
Cool, thanks Linus.
But you're correct in what you say Linus. Srinidhi, the comments which
are you say are confusing are the bits you are the author of:
e208c447bd728920e4f3d438a706344ea31249b9?
yes, it is of course evident that it's my commit from git.
I think you are not catching my point. What I mean is:
fast mode devices are downward compatible and can communicate
with the devices with standard only mode. Those comments in that
enum says exactly that (with "up to" keyword). However the code
which configures the bus in fast mode and restrict the frequency
to operate in 100 kb/s without any reason, and this creates
confusion..
Perhaps, I think we need to remove one of these
parameters as configurable options and derive out the
frequency based on the mode of operation..
I think Linus is fixing the frequency issues in platform code and I have
done so from Device Tree, so I think we're hot-to-trot.
Linus, Srinidhi,
If you're happy with the patch now (not withstanding the removal of
slsu, which I'll do in a separate patch), can you ack it please? By the
way, which tree will this go through?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
