[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