i2c: cadence: Fix the hold bit setting

Message ID 1544768917-8951-1-git-send-email-shubhrajyoti.datta@xilinx.com
State Superseded
Headers show
Series
  • i2c: cadence: Fix the hold bit setting
Related show

Commit Message

Shubhrajyoti Datta Dec. 14, 2018, 6:28 a.m.
In case the hold bit is not needed we are carrying the old values
fix the same by resetting the bit when receive count is less than the
fifo depth.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/i2c/busses/i2c-cadence.c | 5 +++++
 1 file changed, 5 insertions(+)

--
2.1.1

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Comments

Kyle Roeschley Dec. 14, 2018, 3 p.m. | #1
On Fri, Dec 14, 2018 at 11:58:37AM +0530, Shubhrajyoti Datta wrote:
> In case the hold bit is not needed we are carrying the old values
> fix the same by resetting the bit when receive count is less than the
> fifo depth.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Tested-by: Kyle Roeschley <kyle.roeschley@ni.com>
Wolfram Sang Dec. 17, 2018, 10:57 p.m. | #2
Hi,

On Fri, Dec 14, 2018 at 11:58:37AM +0530, Shubhrajyoti Datta wrote:
> In case the hold bit is not needed we are carrying the old values
> fix the same by resetting the bit when receive count is less than the
> fifo depth.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>


Thanks for this patch, yet I have some questions:

* Is this the response to Kyle's patch "Revert "i2c: cadance: fix
  ctrl/addr reg write order"? If so, that should be mentioned somewhere.

* If so, Kyle should get a Reported-by tag? And a Fixes tag should be
  added?

* Even if this is not related to Kyle's issue, is this a bugfix?

* Furthermore, Michal is listed as the maintainer, so I will wait until
  he acks this patch. Seeing Shubhrajyoti is also from Xilinx, does it
  make sense to add him as a maintainer for this driver?

> This email and any attachments are intended for the sole use of the
> named recipient(s) and contain(s) confidential information that may be
> proprietary, privileged or copyrighted under applicable law. If you
> are not the intended recipient, do not read, copy, or forward this
> email message or any attachments. Delete this email message and any
> attachments immediately.

This makes zero sense on a mailing list. Please remove.

Regards,

   Wolfram
Michal Simek Dec. 18, 2018, 12:45 p.m. | #3
Hi,

On 17. 12. 18 23:57, Wolfram Sang wrote:
> Hi,
> 
> On Fri, Dec 14, 2018 at 11:58:37AM +0530, Shubhrajyoti Datta wrote:
>> In case the hold bit is not needed we are carrying the old values
>> fix the same by resetting the bit when receive count is less than the
>> fifo depth.
>>
>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> 
> Thanks for this patch, yet I have some questions:
> 
> * Is this the response to Kyle's patch "Revert "i2c: cadance: fix
>   ctrl/addr reg write order"? If so, that should be mentioned somewhere.
> 
> * If so, Kyle should get a Reported-by tag? And a Fixes tag should be
>   added?
> 
> * Even if this is not related to Kyle's issue, is this a bugfix?

Shubhrajyoti: Please fix description and send v2.

Thanks,
Michal
Kyle Roeschley Jan. 18, 2019, 2:34 p.m. | #4
Hi Shubhrajyoti,

Any news on this patch? We're still seeing occasional kernel hangs on Zynq
which are fixed by this patch, but I'd like to grab it from master or the i2c
tree if possible.

Thanks,
Wolfram Sang Jan. 18, 2019, 6:36 p.m. | #5
> Any news on this patch? We're still seeing occasional kernel hangs on Zynq
> which are fixed by this patch, but I'd like to grab it from master or the i2c
> tree if possible.

There is a V2:

[PATCHv2] i2c: cadence: Fix the hold bit setting
http://patchwork.ozlabs.org/patch/1022287/

Sadly, you have not been added to the CC list.

Patch

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index b136057..b0e284d 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -384,6 +384,8 @@  static void cdns_i2c_mrecv(struct cdns_i2c *id)
         */
        if (id->recv_count > CDNS_I2C_FIFO_DEPTH)
                ctrl_reg |= CDNS_I2C_CR_HOLD;
+       else
+               ctrl_reg = ctrl_reg & ~CDNS_I2C_CR_HOLD;

        cdns_i2c_writereg(ctrl_reg, CDNS_I2C_CR_OFFSET);

@@ -442,6 +444,9 @@  static void cdns_i2c_msend(struct cdns_i2c *id)
         */
        if (id->send_count > CDNS_I2C_FIFO_DEPTH)
                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. */