How an I2c read as well as write operation in "handler function" of request_threaded_irq affects the driver as a whole.?

I have a driver code with handler function and thread function of request_threaded_irq similar to this:

irq-handler fn()
{
      /*disable device interrupt*/
      i2c read from register;
      set disable bit to client-device-interrupt 
      i2c write back;
      return IRQ_WAKe_THREAD;
}



irq-thread fn()
{
      i2c read from register;
      ....
      ....
      /*enable device interrupt*/
      i2c read from register;
      set enable bit to client-device-interrupt 
      i2c write back;
      /*Rest of the operation*/
      ..........
      ..........
      return IRQ_HANDLED;
}

I have few question with respect to above implentation.

1. Will 2 i2c operation in "handler fn" takes considerable amount of time.?

2. Should I need to make bit manipulation in "handler fn" atomic?

3. Should I move the operation performed till "enable device interrupt" from "thread fn" to "handler fn"(this would cost me 4 more i2c operation and one bit manipulation exactly) ? - reason being chances are there that i can miss interrupt as per above code implementation.

4. If I do so(as per question 3). how does it affects the other device interrupts.(as I have a basic doubt whether "handler fn" in hard IRQ context operates with interrupts disabled)

Please provide me a good and optimum solution for the above scenario.

Thanks in advance.

I2C read/write transfers are NOT deterministic.

The protocol allows peripheral slave ICs to perform clock stretching thereby allowing them to "hold" the master until they are ready. However this is NOT a common scenario and hence each I2C transfer usually completes in a pre-determined interval most of the time. However, it is NOT a guarantee, and hence NOT a good idea to perform several I2C transfers within an ISR.

This link contains a nice explanation about the fundamentals of threaded irqs and their proper usage.

---

Optimal design for the above scenario ?

Using threaded-interrupt handler approach will not have many benefits as attempting to enable/disable the interrupts on the device will add to the latency.

In your current scenario (single interrupt from single device), one can stick to the regular request_irq() to register an interrupt service routine(ISR).

ISR code :
1. In the ISR, call disable_irq() to prevent further interrupts.
2. Schedule a bottom half handler function (workqueue is a good choice).
3. Return IRQ_HANDLED from the ISR.

Bottom-half handler code :
4. Perform I2C transfers.
5. Call enable_irq() and exit.

---

NOTE :
Another way to implement the same design would be to use a threaded-irq without an ISR. This achieves the same as the above design and eliminates the need to define/initialise/cleanup the bottom-half handler separately in your code.

In this approach one would put all the I2C read/write code within the IRQ thread function and pass it to request_threaded_irq() along-with handler = NULL i.e. an empty ISR.