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. >>>> +} -- 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