diff mbox series

[v1,6/6] i2c: iproc: handle rx fifo full interrupt

Message ID 20201011182254.17776-7-rayagonda.kokatanur@broadcom.com
State Accepted
Commit 4d658451c9d620ab8fb79483bf83d65b14375723
Headers show
Series fix iproc driver to handle master read request | expand

Commit Message

Rayagonda Kokatanur Oct. 11, 2020, 6:22 p.m. UTC
Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
master write request with >= 64 bytes.

Iproc has a slave rx fifo size of 64 bytes.
Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
when RX fifo becomes full. This can happen if master issues write
request of more than 64 bytes.

Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
---
 drivers/i2c/busses/i2c-bcm-iproc.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

Dhananjay Phadke Oct. 12, 2020, 10:03 p.m. UTC | #1
From: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>

On Sun, 11 Oct 2020 23:52:54 +0530, Rayagonda Kokatanur wrote:
> Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
> master write request with >= 64 bytes.
> 
> Iproc has a slave rx fifo size of 64 bytes.
> Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
> when RX fifo becomes full. This can happen if master issues write
> request of more than 64 bytes.
> 

ARM cores run much faster than I2C bus, why would rx fifo go full when
rx interrupt is enabled and bytes are read out by bus driver isr?
Isn't fifo read pointer updated on these byte reads?
Does controller stretch clock when rx fifo is full (e.g. kernel has
crashed, bus driver isn't draining fifo)?
Ray Jui Oct. 23, 2020, 5:42 p.m. UTC | #2
On 10/12/2020 3:03 PM, Dhananjay Phadke wrote:
> From: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> 
> On Sun, 11 Oct 2020 23:52:54 +0530, Rayagonda Kokatanur wrote:
>> Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
>> master write request with >= 64 bytes.
>>
>> Iproc has a slave rx fifo size of 64 bytes.
>> Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
>> when RX fifo becomes full. This can happen if master issues write
>> request of more than 64 bytes.
>>
> 
> ARM cores run much faster than I2C bus, why would rx fifo go full when
> rx interrupt is enabled and bytes are read out by bus driver isr?
> Isn't fifo read pointer updated on these byte reads?

Hi Rayagonda,

Could you please reply on this question? For transactions > 64 bytes, do
we batch until RX FIFO is full before we read out the data?

Thanks,

Ray

> Does controller stretch clock when rx fifo is full (e.g. kernel has
> crashed, bus driver isn't draining fifo)?
>
Rayagonda Kokatanur Oct. 26, 2020, 3:13 p.m. UTC | #3
Hi Dhanajay,

On Fri, Oct 23, 2020 at 11:12 PM Ray Jui <ray.jui@broadcom.com> wrote:
>
>
>
> On 10/12/2020 3:03 PM, Dhananjay Phadke wrote:
> > From: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> >
> > On Sun, 11 Oct 2020 23:52:54 +0530, Rayagonda Kokatanur wrote:
> >> Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
> >> master write request with >= 64 bytes.
> >>
> >> Iproc has a slave rx fifo size of 64 bytes.
> >> Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
> >> when RX fifo becomes full. This can happen if master issues write
> >> request of more than 64 bytes.
> >>
> >
> > ARM cores run much faster than I2C bus, why would rx fifo go full when
> > rx interrupt is enabled and bytes are read out by bus driver isr?
> > Isn't fifo read pointer updated on these byte reads?
>
> Hi Rayagonda,
>
> Could you please reply on this question? For transactions > 64 bytes, do
> we batch until RX FIFO is full before we read out the data?

Sorry I missed this question.
Yes with current design we are batching 64 bytes for translation > 64 bytes.

Best regards,
Rayagonda


>
> Thanks,
>
> Ray
>
> > Does controller stretch clock when rx fifo is full (e.g. kernel has
> > crashed, bus driver isn't draining fifo)?
> >
Ray Jui Oct. 27, 2020, 12:36 a.m. UTC | #4
On 10/26/2020 8:13 AM, Rayagonda Kokatanur wrote:
> Hi Dhanajay,
> 
> On Fri, Oct 23, 2020 at 11:12 PM Ray Jui <ray.jui@broadcom.com> wrote:
>>
>>
>>
>> On 10/12/2020 3:03 PM, Dhananjay Phadke wrote:
>>> From: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
>>>
>>> On Sun, 11 Oct 2020 23:52:54 +0530, Rayagonda Kokatanur wrote:
>>>> Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
>>>> master write request with >= 64 bytes.
>>>>
>>>> Iproc has a slave rx fifo size of 64 bytes.
>>>> Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
>>>> when RX fifo becomes full. This can happen if master issues write
>>>> request of more than 64 bytes.
>>>>
>>>
>>> ARM cores run much faster than I2C bus, why would rx fifo go full when
>>> rx interrupt is enabled and bytes are read out by bus driver isr?
>>> Isn't fifo read pointer updated on these byte reads?
>>
>> Hi Rayagonda,
>>
>> Could you please reply on this question? For transactions > 64 bytes, do
>> we batch until RX FIFO is full before we read out the data?
> 
> Sorry I missed this question.
> Yes with current design we are batching 64 bytes for translation > 64 bytes.
> 

So we do batch the transfer and read them in one shot, and that's how
the FIFO full interrupt is being utilized for. That sounds okay to me.

Thanks,

Ray

> Best regards,
> Rayagonda
> 
> 
>>
>> Thanks,
>>
>> Ray
>>
>>> Does controller stretch clock when rx fifo is full (e.g. kernel has
>>> crashed, bus driver isn't draining fifo)?
>>>
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index 22e04055b447..cceaf69279a9 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -313,6 +313,8 @@  static void bcm_iproc_i2c_slave_init(
 
 	/* Enable interrupt register to indicate a valid byte in receive fifo */
 	val = BIT(IE_S_RX_EVENT_SHIFT);
+	/* Enable interrupt register to indicate Slave Rx FIFO Full */
+	val |= BIT(IE_S_RX_FIFO_FULL_SHIFT);
 	/* Enable interrupt register to indicate a Master read transaction */
 	val |= BIT(IE_S_RD_EVENT_SHIFT);
 	/* Enable interrupt register for the Slave BUSY command */
@@ -434,9 +436,15 @@  static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
 	 *                    events
 	 * Master-read      : both IS_S_RX_EVENT_SHIFT and IS_S_RD_EVENT_SHIFT
 	 *                    events or only IS_S_RD_EVENT_SHIFT
+	 *
+	 * iproc has a slave rx fifo size of 64 bytes. Rx fifo full interrupt
+	 * (IS_S_RX_FIFO_FULL_SHIFT) will be generated when RX fifo becomes
+	 * full. This can happen if Master issues write requests of more than
+	 * 64 bytes.
 	 */
 	if (status & BIT(IS_S_RX_EVENT_SHIFT) ||
-	    status & BIT(IS_S_RD_EVENT_SHIFT)) {
+	    status & BIT(IS_S_RD_EVENT_SHIFT) ||
+	    status & BIT(IS_S_RX_FIFO_FULL_SHIFT)) {
 		/* disable slave interrupts */
 		val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
 		val &= ~iproc_i2c->slave_int_mask;
@@ -452,9 +460,14 @@  static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* schedule tasklet to read data later */
 		tasklet_schedule(&iproc_i2c->slave_rx_tasklet);
 
-		/* clear only IS_S_RX_EVENT_SHIFT interrupt */
-		iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET,
-				 BIT(IS_S_RX_EVENT_SHIFT));
+		/*
+		 * clear only IS_S_RX_EVENT_SHIFT and
+		 * IS_S_RX_FIFO_FULL_SHIFT interrupt.
+		 */
+		val = BIT(IS_S_RX_EVENT_SHIFT);
+		if (status & BIT(IS_S_RX_FIFO_FULL_SHIFT))
+			val |= BIT(IS_S_RX_FIFO_FULL_SHIFT);
+		iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, val);
 	}
 
 	if (status & BIT(IS_S_TX_UNDERRUN_SHIFT)) {