[PATCH linux dev-4.13] i2c: fsi: Don't take a lock around FSI accesses

[Benjamin Herrenschmidt]

On Tue, 2018-05-29 at 10:11 -0500, Eddie James wrote:
> 
> On 05/28/2018 11:28 PM, Benjamin Herrenschmidt wrote:
> > That lock is incorrect as FSI can sleep and thus will
> > trigger lockdep warnings. It is also unnecessary as
> > it was meant to "speed up" the reset process for
> > unspecified reasons, not to protect against anything.
> > 
> > The the master semaphore should prevent any concurrent user from
> > seeing the intermediate state already and the recent improvements
> > to the FSI layer should take care of potential speed issues.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> > ---
> >   drivers/i2c/busses/i2c-fsi.c | 8 ++------
> >   1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> > index 10f693fc855b..41cbffea4325 100644
> > --- a/drivers/i2c/busses/i2c-fsi.c
> > +++ b/drivers/i2c/busses/i2c-fsi.c
> > @@ -23,6 +23,7 @@
> >   #include <linux/semaphore.h>
> >   #include <linux/spinlock.h>
> >   #include <linux/wait.h>
> > +#include <linux/delay.h>
> 
> delay.h was already included :)
> 
> And probably need to remove the spinlock from the master structure, as 
> it's unused.

Ah right.

Cheers,
Ben.

> Thanks,
> Eddie
> 
> > 
> >   #define FSI_ENGID_I2C		0x7
> > 
> > @@ -427,10 +428,6 @@ static int fsi_i2c_reset(struct fsi_i2c_master *i2c, u16 port)
> >   {
> >   	int rc;
> >   	u32 mode, stat, dummy = 0;
> > -	unsigned long flags;
> > -
> > -	/* disable pre-emption; bus won't get left in a bad state for long */
> > -	spin_lock_irqsave(&i2c->reset_lock, flags);
> > 
> >   	/* reset engine */
> >   	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_I2C, &dummy);
> > @@ -472,7 +469,7 @@ static int fsi_i2c_reset(struct fsi_i2c_master *i2c, u16 port)
> >   		goto done;
> > 
> >   	/* wait for command complete; time from legacy driver */
> > -	udelay(1000);
> > +	msleep(1);
> > 
> >   	rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_STAT, &stat);
> >   	if (rc)
> > @@ -490,7 +487,6 @@ static int fsi_i2c_reset(struct fsi_i2c_master *i2c, u16 port)
> >   	rc = fsi_i2c_dev_init(i2c);
> > 
> >   done:
> > -	spin_unlock_irqrestore(&i2c->reset_lock, flags);
> >   	return rc;
> >   }
> >