On Tue, Feb 05, 2019 at 01:32:51PM +0100, Wolfram Sang wrote:
> On Tue, Feb 05, 2019 at 04:42:53PM +0530, shubhrajyoti.datta@xxxxxxxxx wrote:
> > From: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxxxxx>
> >
> > In case the hold bit is not needed we are carrying 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.
> >
> >
> > Fixes: df8eb5691c48 ("i2c: Add driver for Cadence I2C controller")
> > Acked-by: Michal Simek <michal.simek@xxxxxxxxxx>
> > Reported-and-tested-by: Kyle Roeschley <kyle.roeschley@xxxxxx>
>
> Since code was changed since v4, I'd like to ask Kyle to retest this
> version of the patch, too.
>
I re-tested and this still fixes our bus timeouts and system hang.
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxxxxx>
> > ---
> > v5:
> > fix some typos
>
> This is too lazy. Please describe exactly what you changed. Otherwise me
> or the reviewers have to look it up. No need to resend because of this,
> but I am getting a little annoyed with this patch because each version
> feels rushed in some way wasting others people time.
>
> [...]
> > - if (id->send_count > CDNS_I2C_FIFO_DEPTH)
> > + if ((id->send_count > CDNS_I2C_FIFO_DEPTH) || id->bus_hold_flag)
> > ctrl_reg |= CDNS_I2C_CR_HOLD;
> > + else
> > + ctrl_reg = ctrl_reg & ~CDNS_I2C_CR_HOLD;
We should use &= to be consistent. Peter's comment on v5 already mentioned
this, but after v6 was sent.
--
Kyle Roeschley
Software Engineer
National Instruments
