lkml.org 
[lkml]   [2013]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v5 1/4] i2c: busses: i2c-st: Add ST I2C controller

On 10/17/2013 04:16 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:27 Thu 17 Oct , Maxime COQUELIN wrote:
>> Hi Jean-Christophe,
>>
>> On 10/16/2013 05:14 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>>
>> ...
>>>> +
>>>> +static inline void st_i2c_set_bits(void __iomem *reg, u32 mask)
>>>> +{
>>>> + writel(readl(reg) | mask, reg);
>>>> +}
>>>> +
>>>> +static inline void st_i2c_clr_bits(void __iomem *reg, u32 mask)
>>>> +{
>>>> + writel(readl(reg) & ~mask, reg);
>>> use set_bit api and use relaxed version
>> Using the set_bit api here does not match with the purpose of these
>> functions.
>> We want to be able to set/clear multiple bits, and AFAICS the set_bit
>> api does not
>> provide this possibility.
>>
>> I took example on i2c-nomadik for these functions.
>>
> so factorize the code not copy and paste
I won't create a new API for this.
If this is blocking for you, then I will just remove this functions.

>>>> +}
>>>> +
>>>> +/* From I2C Specifications v0.5 */
>>>> +static struct st_i2c_timings i2c_timings[] = {
>>>> use readsl
>> Since the read content is flushed, I prefer keeping it as it is instead
>> of allocating
>> a buffer of the FIFO's size.
> keep point is to speedup the bus
I meant I will use readl_relaxed, not readl.

>>> use relaxed version as much as possible
>> I was not comfortable with the different possibilities (_raw_readl,
>> readl_relaxed, readl...).
>> I found this interresting discussion:
>> _http://comments.gmane.org/gmane.linux.ports.arm.kernel/117626
>> From what I understood, you are right, I should be able to use
>> readl_relaxed everywhere.
>>
>> Maybe I should perform a readl on the interrupt mask register before
>> returning from the interrupt handler,
>> in order to ensure that the write to the IEN register is effective
>> before the IRQ for the device is re-enabled at GIC level.
>> Maybe this could avoid the few spurious interrupts I face sometimes, I
>> will have a try.
> ok
I failed to reproduce the spurious interrupt without the patch, so I
can't say
whether performing a readl at this stage helps.
>>>> +}


\
 
 \ /
  Last update: 2013-10-17 19:41    [W:0.100 / U:0.720 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site