Re: [PATCH v2 4/7] i2c: omap: in case of VERSION_2 read IRQSTATUS_RAW but write to IRQSTATUS

[Santosh Shilimkar <santosh.shilimkar@ti.com>]

On Thursday 25 October 2012 06:22 PM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Oct 25, 2012 at 06:23:57PM +0530, Santosh Shilimkar wrote:
>> On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:
>>> on OMAP4+ we want to read IRQSTATUS_RAW register,
>>> instead of IRQSTATUS. The reason being that IRQSTATUS
>>> will only contain the bits which were enabled on
>>> IRQENABLE_SET and that will break when we need to
>>> poll for a certain bit which wasn't enabled as an
>>> IRQ source.
>>>
>>> One such case is after we finish converting to
>>> deferred stop bit, we will have to poll for ARDY
>>> bit before returning control for the client driver
>>> in order to prevent us from trying to start a
>>> transfer on a bus which is already busy.
>>>
>>> Note, however, that omap-i2c.c needs a big rework
>>> on register definition and register access. Such
>>> work will be done in a separate series of patches.
>>>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>>> ---
>>>   drivers/i2c/busses/i2c-omap.c | 12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
>>> index b004126..20f9ad6 100644
>>> --- a/drivers/i2c/busses/i2c-omap.c
>>> +++ b/drivers/i2c/busses/i2c-omap.c
>>> @@ -271,8 +271,18 @@ static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
>>>
>>>   static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
>>>   {
>>> -	return __raw_readw(i2c_dev->base +
>>> +	/* if we are OMAP_I2C_IP_VERSION_2, we need to read from
>>> +	 * IRQSTATUS_RAW, but write to IRQSTATUS
>>> +	 */
>>> +	if ((i2c_dev->dtrev == OMAP_I2C_IP_VERSION_2) &&
>>> +			(reg == OMAP_I2C_STAT_REG)) {
>> Doing this check on every I2C register read seems to
>> expensive to me. Can you not sort this in init with some offset
>> which can be 0 or non zero ?  Sorry in case this is already dicussed.
>
> could be. I didn't go that route because I'm planning a complete rewrite
> of all register accesses. The way it's done today is completely broken
> and already expensive (with reg_shift and different map tables and so
> on).
>
> If it's really a big of a deal, I can try to find another way, maybe
> just adding omap_i2c_read_stat() and limit the version check just to
> I2C_STAT reads would do it for now...
>
Its a hot path since you read many I2C register reads, so getting
rid of that additional check will be good.