diff mbox series

[PATCHv5] i2c: cadence: Fix the driver in interrupt flurry case

Message ID 1574332651-17308-1-git-send-email-shubhrajyoti.datta@gmail.com
State Superseded
Headers show
Series [PATCHv5] i2c: cadence: Fix the driver in interrupt flurry case | expand

Commit Message

Shubhrajyoti Datta Nov. 21, 2019, 10:37 a.m. UTC 
From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

In case there are a lot of interrupts from a non i2c peripheral
the  clear is not released and the timeout is reached, then the hold
bit is not released.
Protect the code by making it atomic by disabling interrupts.

Fixes: df8eb5691c4 ("i2c: Add driver for Cadence I2C controller")
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v2: Update the wording
v3: Add fixes tag
v4: update fixes tag
v5: Update the description

 drivers/i2c/busses/i2c-cadence.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Michal Simek June 17, 2020, 11:54 a.m. UTC | #1 
Hi,

On 21. 11. 19 11:37, shubhrajyoti.datta@gmail.com wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> In case there are a lot of interrupts from a non i2c peripheral
> the  clear is not released and the timeout is reached, then the hold
> bit is not released.
> Protect the code by making it atomic by disabling interrupts.
> 
> Fixes: df8eb5691c4 ("i2c: Add driver for Cadence I2C controller")
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2: Update the wording
> v3: Add fixes tag
> v4: update fixes tag
> v5: Update the description
> 
>  drivers/i2c/busses/i2c-cadence.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index b136057..92829be 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -365,6 +365,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>  {
>  	unsigned int ctrl_reg;
>  	unsigned int isr_status;
> +	unsigned long flags;
>  
>  	id->p_recv_buf = id->p_msg->buf;
>  	id->recv_count = id->p_msg->len;
> @@ -405,6 +406,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>  		cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
>  	}
>  
> +	local_irq_save(flags);
>  	/* Set the slave address in address register - triggers operation */
>  	cdns_i2c_writereg(id->p_msg->addr & CDNS_I2C_ADDR_MASK,
>  						CDNS_I2C_ADDR_OFFSET);
> @@ -413,6 +415,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>  		((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
>  		(id->recv_count <= CDNS_I2C_FIFO_DEPTH))
>  			cdns_i2c_clear_bus_hold(id);
> +	local_irq_restore(flags);
>  	cdns_i2c_writereg(CDNS_I2C_ENABLED_INTR_MASK, CDNS_I2C_IER_OFFSET);
>  }
>  
> 

As Marek pointed out this is not the right way how to do protect the
code. local_irq_save/restore just disable IRQ on certain CPU which is
likely what it is not going to work on SMP systems.

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index b136057..92829be 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -365,6 +365,7 @@  static void cdns_i2c_mrecv(struct cdns_i2c *id)
 {
 	unsigned int ctrl_reg;
 	unsigned int isr_status;
+	unsigned long flags;
 
 	id->p_recv_buf = id->p_msg->buf;
 	id->recv_count = id->p_msg->len;
@@ -405,6 +406,7 @@  static void cdns_i2c_mrecv(struct cdns_i2c *id)
 		cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
 	}
 
+	local_irq_save(flags);
 	/* Set the slave address in address register - triggers operation */
 	cdns_i2c_writereg(id->p_msg->addr & CDNS_I2C_ADDR_MASK,
 						CDNS_I2C_ADDR_OFFSET);
@@ -413,6 +415,7 @@  static void cdns_i2c_mrecv(struct cdns_i2c *id)
 		((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
 		(id->recv_count <= CDNS_I2C_FIFO_DEPTH))
 			cdns_i2c_clear_bus_hold(id);
+	local_irq_restore(flags);
 	cdns_i2c_writereg(CDNS_I2C_ENABLED_INTR_MASK, CDNS_I2C_IER_OFFSET);
 }