[PATCHv2] i2c: cadence: Fix the hold bit setting

Message ID 1547012367-26581-1-git-send-email-shubhrajyoti.datta@gmail.com
State Superseded
Headers show
Series
  • [PATCHv2] i2c: cadence: Fix the hold bit setting
Related show

Commit Message

Shubhrajyoti Datta Jan. 9, 2019, 5:39 a.m.
From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

In case the hold bit is not needed we are carring the old values
fix the same by resetting the bit when not needed.

Fixes the sporadic i2c bus lockups on National Instruments
Zynq-based devices.

Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v2:
Update commit log
Also check if the hold_flag

 drivers/i2c/busses/i2c-cadence.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Michal Simek Jan. 9, 2019, 10:06 a.m. | #1
On 09. 01. 19 6:39, shubhrajyoti.datta@gmail.com wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> In case the hold bit is not needed we are carring the old values
> fix the same by resetting the bit when not needed.
> 
> Fixes the sporadic i2c bus lockups on National Instruments
> Zynq-based devices.
> 
> Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2:
> Update commit log
> Also check if the hold_flag

Kyle: Can you please retest it?

Thanks,
Michal
Wolfram Sang Jan. 9, 2019, 10:43 a.m. | #2
On Wed, Jan 09, 2019 at 11:09:27AM +0530, shubhrajyoti.datta@gmail.com wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> In case the hold bit is not needed we are carring the old values
> fix the same by resetting the bit when not needed.
> 
> Fixes the sporadic i2c bus lockups on National Instruments
> Zynq-based devices.
> 
> Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Is there a suitable Fixes tag?
Kyle Roeschley Jan. 18, 2019, 7:25 p.m. | #3
On Wed, Jan 09, 2019 at 10:06:59AM +0000, Michal Simek wrote:
> On 09. 01. 19 6:39, shubhrajyoti.datta@gmail.com wrote:
> > From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > 
> > In case the hold bit is not needed we are carring the old values
> > fix the same by resetting the bit when not needed.
> > 
> > Fixes the sporadic i2c bus lockups on National Instruments
> > Zynq-based devices.
> > 
> > Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
> > v2:
> > Update commit log
> > Also check if the hold_flag
> 
> Kyle: Can you please retest it?
> 
> Thanks,
> Micha

Tested and works great.
Michal Simek Jan. 21, 2019, 7:23 a.m. | #4
On 09. 01. 19 11:43, Wolfram Sang wrote:
> On Wed, Jan 09, 2019 at 11:09:27AM +0530, shubhrajyoti.datta@gmail.com wrote:
>> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>>
>> In case the hold bit is not needed we are carring the old values
>> fix the same by resetting the bit when not needed.
>>
>> Fixes the sporadic i2c bus lockups on National Instruments
>> Zynq-based devices.
>>
>> Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> Is there a suitable Fixes tag?

Shubhrajyoti: Is there any patch which is fixed by this patch?

Thanks,
Michal
Shubhrajyoti Datta Jan. 21, 2019, 10:23 a.m. | #5
On Mon, Jan 21, 2019 at 12:54 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 09. 01. 19 11:43, Wolfram Sang wrote:
> > On Wed, Jan 09, 2019 at 11:09:27AM +0530, shubhrajyoti.datta@gmail.com wrote:
> >> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> >>
> >> In case the hold bit is not needed we are carring the old values
> >> fix the same by resetting the bit when not needed.
> >>
> >> Fixes the sporadic i2c bus lockups on National Instruments
> >> Zynq-based devices.
> >>
> >> Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
> >> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> >
> > Is there a suitable Fixes tag?
>
> Shubhrajyoti: Is there any patch which is fixed by this patch?
this issue  is there since  early

df8eb5691 : i2c: Add driver for Cadence I2C controller
>
> Thanks,
> Michal
>
>
Michal Simek Jan. 21, 2019, 12:22 p.m. | #6
On 21. 01. 19 11:23, Shubhrajyoti Datta wrote:
> On Mon, Jan 21, 2019 at 12:54 PM Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> On 09. 01. 19 11:43, Wolfram Sang wrote:
>>> On Wed, Jan 09, 2019 at 11:09:27AM +0530, shubhrajyoti.datta@gmail.com wrote:
>>>> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>>>>
>>>> In case the hold bit is not needed we are carring the old values
>>>> fix the same by resetting the bit when not needed.
>>>>
>>>> Fixes the sporadic i2c bus lockups on National Instruments
>>>> Zynq-based devices.
>>>>
>>>> Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
>>>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>>>
>>> Is there a suitable Fixes tag?
>>
>> Shubhrajyoti: Is there any patch which is fixed by this patch?
> this issue  is there since  early
> 
> df8eb5691 : i2c: Add driver for Cadence I2C controller

Ok. It means please send v3 with this tag there and also please add my
Acked-by: Michal Simek <michal.simek@xilinx.com>

Then Wolfram will pick it.

Thanks,
Michal

Patch

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index b136057..b2b3df1 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -382,8 +382,10 @@  static void cdns_i2c_mrecv(struct cdns_i2c *id)
 	 * Check for the message size against FIFO depth and set the
 	 * 'hold bus' bit if it is greater than FIFO depth.
 	 */
-	if (id->recv_count > CDNS_I2C_FIFO_DEPTH)
+	if (id->recv_count > CDNS_I2C_FIFO_DEPTH  || id->bus_hold_flag)
 		ctrl_reg |= CDNS_I2C_CR_HOLD;
+	else
+		ctrl_reg = ctrl_reg & ~CDNS_I2C_CR_HOLD;
 
 	cdns_i2c_writereg(ctrl_reg, CDNS_I2C_CR_OFFSET);
 
@@ -440,8 +442,11 @@  static void cdns_i2c_msend(struct cdns_i2c *id)
 	 * Check for the message size against FIFO depth and set the
 	 * 'hold bus' bit if it is greater than FIFO depth.
 	 */
-	if (id->send_count > CDNS_I2C_FIFO_DEPTH)
+	if (id->recv_count > CDNS_I2C_FIFO_DEPTH  || id->bus_hold_flag)
 		ctrl_reg |= CDNS_I2C_CR_HOLD;
+	else
+		ctrl_reg = ctrl_reg & ~CDNS_I2C_CR_HOLD;
+
 	cdns_i2c_writereg(ctrl_reg, CDNS_I2C_CR_OFFSET);
 
 	/* Clear the interrupts in interrupt status register. */