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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4FDEE99C.9090203@linaro.org>
Date:	Mon, 18 Jun 2012 09:41:00 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Srinidhi Kasagar <srinidhi.kasagar@...ricsson.com>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Linus WALLEIJ <linus.walleij@...ricsson.com>,
	"arnd@...db.de" <arnd@...db.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: Re: Fwd: [PATCH 1/3] i2c: Add Device Tree support to the Nomadik
 I2C driver

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@...aro.org>   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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/