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

Eddie James eajames at linux.vnet.ibm.com
Wed May 30 01:11:51 AEST 2018



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.

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;
>   }
>



More information about the openbmc mailing list