diff mbox series

[v3,3/3] spi: s3c64xx: support interrupt based pio mode

Message ID 20230502062813.112434-4-jaewon02.kim@samsung.com
State Accepted
Commit 1ee806718d5ef7de31c6063c4493f3d6527c9427
Headers show
Series [v3,1/3] spi: s3c64xx: change polling mode to optional | expand

Commit Message

Jaewon Kim May 2, 2023, 6:28 a.m. UTC
Support interrupt based pio mode to optimize cpu usage.
When transmitting data size is larget than 32 bytes, operates with
interrupt based pio mode.

By using the FIFORDY INT, an interrupt can be triggered when
the desired size of data has been received. Using this, we can support
interrupt based pio mode.

Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 66 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 58 insertions(+), 8 deletions(-)

Comments

Krzysztof Kozlowski May 5, 2023, 9:47 a.m. UTC | #1
On 02/05/2023 08:28, Jaewon Kim wrote:
> Support interrupt based pio mode to optimize cpu usage.
> When transmitting data size is larget than 32 bytes, operates with
> interrupt based pio mode.
> 
> By using the FIFORDY INT, an interrupt can be triggered when
> the desired size of data has been received. Using this, we can support
> interrupt based pio mode.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 66 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 58 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 2a8304678df9..323c6da9730b 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -58,6 +58,8 @@
>  #define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD	(1<<17)
>  #define S3C64XX_SPI_MODE_BUS_TSZ_WORD		(2<<17)
>  #define S3C64XX_SPI_MODE_BUS_TSZ_MASK		(3<<17)
> +#define S3C64XX_SPI_MODE_RX_RDY_LVL		GENMASK(16, 11)
> +#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT	11
>  #define S3C64XX_SPI_MODE_SELF_LOOPBACK		(1<<3)
>  #define S3C64XX_SPI_MODE_RXDMA_ON		(1<<2)
>  #define S3C64XX_SPI_MODE_TXDMA_ON		(1<<1)
> @@ -114,6 +116,8 @@
>  
>  #define S3C64XX_SPI_TRAILCNT		S3C64XX_SPI_MAX_TRAILCNT
>  
> +#define S3C64XX_SPI_POLLING_SIZE	32
> +
>  #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
>  #define is_polling(x)	(x->cntrlr_info->polling)
>  
> @@ -552,7 +556,7 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
>  }
>  
>  static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
> -				struct spi_transfer *xfer)
> +				struct spi_transfer *xfer, bool use_irq)
>  {
>  	void __iomem *regs = sdd->regs;
>  	unsigned long val;
> @@ -573,6 +577,12 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
>  	if (RX_FIFO_LVL(status, sdd) < xfer->len)
>  		usleep_range(time_us / 2, time_us);
>  
> +	if (use_irq) {
> +		val = msecs_to_jiffies(ms);
> +		if (!wait_for_completion_timeout(&sdd->xfer_completion, val))
> +			return -EIO;
> +	}
> +
>  	val = msecs_to_loops(ms);
>  	do {
>  		status = readl(regs + S3C64XX_SPI_STATUS);
> @@ -735,10 +745,13 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
>  	void *rx_buf = NULL;
>  	int target_len = 0, origin_len = 0;
>  	int use_dma = 0;
> +	bool use_irq = false;
>  	int status;
>  	u32 speed;
>  	u8 bpw;
>  	unsigned long flags;
> +	u32 rdy_lv;
> +	u32 val;
>  
>  	reinit_completion(&sdd->xfer_completion);
>  
> @@ -759,17 +772,46 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
>  	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
>  		use_dma = 1;
>  
> -	} else if (xfer->len > fifo_len) {
> +	} else if (xfer->len >= fifo_len) {

I don't fully understand this. If len equals to fifo_len, everything
would fit into FIFO so no need for all this?

>  		tx_buf = xfer->tx_buf;
>  		rx_buf = xfer->rx_buf;
>  		origin_len = xfer->len;
> -
>  		target_len = xfer->len;
> -		if (xfer->len > fifo_len)
> -			xfer->len = fifo_len;
> +		xfer->len = fifo_len - 1;
>  	}
>  
>  	do {
> +		/* transfer size is greater than 32, change to IRQ mode */
> +		if (xfer->len > S3C64XX_SPI_POLLING_SIZE)
> +			use_irq = true;
> +
> +		if (use_irq) {
> +			reinit_completion(&sdd->xfer_completion);
> +
> +			rdy_lv = xfer->len;

Style is:

/*
 *

> +			/* Setup RDY_FIFO trigger Level
> +			 * RDY_LVL =
> +			 * fifo_lvl up to 64 byte -> N bytes
> +			 *               128 byte -> RDY_LVL * 2 bytes
> +			 *               256 byte -> RDY_LVL * 4 bytes

I don't understand it. Based on this equation for 256 bytes,
RDY_LVL = RDY_LVL * 4?
Didn't you mean xfer->len?


Best regards,
Krzysztof
Jaewon Kim May 8, 2023, 1:42 a.m. UTC | #2
On 23. 5. 5. 18:47, Krzysztof Kozlowski wrote:
> On 02/05/2023 08:28, Jaewon Kim wrote:
>> Support interrupt based pio mode to optimize cpu usage.
>> When transmitting data size is larget than 32 bytes, operates with
>> interrupt based pio mode.
>>
>> By using the FIFORDY INT, an interrupt can be triggered when
>> the desired size of data has been received. Using this, we can support
>> interrupt based pio mode.
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> ---
>>   drivers/spi/spi-s3c64xx.c | 66 ++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 58 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index 2a8304678df9..323c6da9730b 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -58,6 +58,8 @@
>>   #define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD	(1<<17)
>>   #define S3C64XX_SPI_MODE_BUS_TSZ_WORD		(2<<17)
>>   #define S3C64XX_SPI_MODE_BUS_TSZ_MASK		(3<<17)
>> +#define S3C64XX_SPI_MODE_RX_RDY_LVL		GENMASK(16, 11)
>> +#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT	11
>>   #define S3C64XX_SPI_MODE_SELF_LOOPBACK		(1<<3)
>>   #define S3C64XX_SPI_MODE_RXDMA_ON		(1<<2)
>>   #define S3C64XX_SPI_MODE_TXDMA_ON		(1<<1)
>> @@ -114,6 +116,8 @@
>>   
>>   #define S3C64XX_SPI_TRAILCNT		S3C64XX_SPI_MAX_TRAILCNT
>>   
>> +#define S3C64XX_SPI_POLLING_SIZE	32
>> +
>>   #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
>>   #define is_polling(x)	(x->cntrlr_info->polling)
>>   
>> @@ -552,7 +556,7 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
>>   }
>>   
>>   static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
>> -				struct spi_transfer *xfer)
>> +				struct spi_transfer *xfer, bool use_irq)
>>   {
>>   	void __iomem *regs = sdd->regs;
>>   	unsigned long val;
>> @@ -573,6 +577,12 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
>>   	if (RX_FIFO_LVL(status, sdd) < xfer->len)
>>   		usleep_range(time_us / 2, time_us);
>>   
>> +	if (use_irq) {
>> +		val = msecs_to_jiffies(ms);
>> +		if (!wait_for_completion_timeout(&sdd->xfer_completion, val))
>> +			return -EIO;
>> +	}
>> +
>>   	val = msecs_to_loops(ms);
>>   	do {
>>   		status = readl(regs + S3C64XX_SPI_STATUS);
>> @@ -735,10 +745,13 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
>>   	void *rx_buf = NULL;
>>   	int target_len = 0, origin_len = 0;
>>   	int use_dma = 0;
>> +	bool use_irq = false;
>>   	int status;
>>   	u32 speed;
>>   	u8 bpw;
>>   	unsigned long flags;
>> +	u32 rdy_lv;
>> +	u32 val;
>>   
>>   	reinit_completion(&sdd->xfer_completion);
>>   
>> @@ -759,17 +772,46 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
>>   	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
>>   		use_dma = 1;
>>   
>> -	} else if (xfer->len > fifo_len) {
>> +	} else if (xfer->len >= fifo_len) {
> I don't fully understand this. If len equals to fifo_len, everything
> would fit into FIFO so no need for all this?

If the FIFO is filled with data, TX Overrun & RX Underrun interrupts 
will occur.

In CPU polling, there is no such issue because data is read before an 
interrupt occurs.

And, RDY_LVL has only 6 bits.(max. 63). we cannot set trigger level on 
the FIFO max size.


>>   		tx_buf = xfer->tx_buf;
>>   		rx_buf = xfer->rx_buf;
>>   		origin_len = xfer->len;
>> -
>>   		target_len = xfer->len;
>> -		if (xfer->len > fifo_len)
>> -			xfer->len = fifo_len;
>> +		xfer->len = fifo_len - 1;
>>   	}
>>   
>>   	do {
>> +		/* transfer size is greater than 32, change to IRQ mode */
>> +		if (xfer->len > S3C64XX_SPI_POLLING_SIZE)
>> +			use_irq = true;
>> +
>> +		if (use_irq) {
>> +			reinit_completion(&sdd->xfer_completion);
>> +
>> +			rdy_lv = xfer->len;
> Style is:
>
> /*
>   *
>
>> +			/* Setup RDY_FIFO trigger Level
>> +			 * RDY_LVL =
>> +			 * fifo_lvl up to 64 byte -> N bytes
>> +			 *               128 byte -> RDY_LVL * 2 bytes
>> +			 *               256 byte -> RDY_LVL * 4 bytes
> I don't understand it. Based on this equation for 256 bytes,
> RDY_LVL = RDY_LVL * 4?
> Didn't you mean xfer->len?

In v4, I will change it to the following

/*
  * Trigger Level =
  * (N = value of RDY_LVL field)
  * fifo_lvl up to 64 byte -> N bytes
  *               128 byte -> N * 2 bytes
  *               256 byte -> N * 4 bytes
  */

>
>
> Best regards,
> Krzysztof
>
>

Thanks

Jaewon Kim
Marek Szyprowski May 9, 2023, 1:03 p.m. UTC | #3
On 02.05.2023 08:28, Jaewon Kim wrote:
> Support interrupt based pio mode to optimize cpu usage.
> When transmitting data size is larget than 32 bytes, operates with
> interrupt based pio mode.
>
> By using the FIFORDY INT, an interrupt can be triggered when
> the desired size of data has been received. Using this, we can support
> interrupt based pio mode.
>
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>

This patch landed recently in linux-next as commit 1ee806718d5e ("spi: 
s3c64xx: support interrupt based pio mode"). Unfortunately it breaks 
ethernet chip operation on Exynos3250 based Artik5 Development board. I 
see the flood of the following messages:

[   36.097739] ax88796c spi0.0: I/O Error: rx-1 tx-0 rx-f tx-p len-496 
dma-1 res-(-5)
[   36.100877] ax88796c spi0.0: RX residue: 248
[   36.101383] ax88796c spi0.0: SPI transfer failed: -5
[   36.101939] spi_master spi0: failed to transfer one message from queue
[   36.102439] ax88796c spi0.0: axspi_read_rxq() failed: ret = -5
[   36.107830] s3c64xx-spi 13920000.spi: Failed to get RX DMA channel
[   36.148875] ax88796c spi0.0: I/O Error: rx-0 tx-1 rx-p tx-f len-4 
dma-0 res-(-5)
[   36.149517] ax88796c spi0.0: SPI transfer failed: -5
[   36.150053] spi_master spi0: failed to transfer one message from queue
[   36.150562] ax88796c spi0.0: axspi_read_reg() failed: ret = -5
[   36.152175] s3c64xx-spi 13920000.spi: Failed to get RX DMA channel
[   36.191651] ax88796c spi0.0: I/O Error: rx-0 tx-1 rx-p tx-f len-4 
dma-0 res-(-5)
[   36.192268] ax88796c spi0.0: SPI transfer failed: -5

...

I didn't analyze the details, but imho it looks like some kind of 
mishandling of the corner case or switching between PIO and DMA mode. I 
will check the details later.


Best regards
Marek Szyprowski May 10, 2023, 6:54 a.m. UTC | #4
On 10.05.2023 06:05, Jaewon Kim wrote:
> Hello Marek
>
>
> On 23. 5. 9. 22:03, Marek Szyprowski wrote:
>> On 02.05.2023 08:28, Jaewon Kim wrote:
>>> Support interrupt based pio mode to optimize cpu usage.
>>> When transmitting data size is larget than 32 bytes, operates with
>>> interrupt based pio mode.
>>>
>>> By using the FIFORDY INT, an interrupt can be triggered when
>>> the desired size of data has been received. Using this, we can support
>>> interrupt based pio mode.
>>>
>>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> This patch landed recently in linux-next as commit 1ee806718d5e ("spi:
>> s3c64xx: support interrupt based pio mode"). Unfortunately it breaks
>> ethernet chip operation on Exynos3250 based Artik5 Development board. I
>> see the flood of the following messages:
>>
>> [   36.097739] ax88796c spi0.0: I/O Error: rx-1 tx-0 rx-f tx-p len-496
>> dma-1 res-(-5)
>> [   36.100877] ax88796c spi0.0: RX residue: 248
>> [   36.101383] ax88796c spi0.0: SPI transfer failed: -5
>> [   36.101939] spi_master spi0: failed to transfer one message from queue
>> [   36.102439] ax88796c spi0.0: axspi_read_rxq() failed: ret = -5
>> [   36.107830] s3c64xx-spi 13920000.spi: Failed to get RX DMA channel
>> [   36.148875] ax88796c spi0.0: I/O Error: rx-0 tx-1 rx-p tx-f len-4
>> dma-0 res-(-5)
>> [   36.149517] ax88796c spi0.0: SPI transfer failed: -5
>> [   36.150053] spi_master spi0: failed to transfer one message from queue
>> [   36.150562] ax88796c spi0.0: axspi_read_reg() failed: ret = -5
>> [   36.152175] s3c64xx-spi 13920000.spi: Failed to get RX DMA channel
>> [   36.191651] ax88796c spi0.0: I/O Error: rx-0 tx-1 rx-p tx-f len-4
>> dma-0 res-(-5)
>> [   36.192268] ax88796c spi0.0: SPI transfer failed: -5
>>
>> ...
>>
>> I didn't analyze the details, but imho it looks like some kind of
>> mishandling of the corner case or switching between PIO and DMA mode. I
>> will check the details later.
>>
>>
>> Best regards
>
> Thanks for testing the various cases.
>
> The problem occurred when DMA mode and IRQ mode were enabled at the same
> time.
>
>
> In the above case, BUS_WIDTH register invaded.
>
> Because, target length 496 were written to RX_RDY_LVL, but it exceeded
> 6-bits.
>
> Could you test with below code??? If the problem is solved, I will send
> a fix patch as soon as possible.

Thanks for quick response. Indeed, the proposed patch fixed the issue!

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> -----------------------------
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 238db29fc93b..a72e11e965c3 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -782,7 +782,7 @@ static int s3c64xx_spi_transfer_one(struct
> spi_master *master,
>
>           do {
>                   /* transfer size is greater than 32, change to IRQ mode */
> -               if (xfer->len > S3C64XX_SPI_POLLING_SIZE)
> +               if (!use_dma && (xfer->len > S3C64XX_SPI_POLLING_SIZE))

Parentheses around 'xfer->len > S3C64XX_SPI_POLLING_SIZE' are not needed.

>                           use_irq = true;
>
>                   if (use_irq) {
>
> -----------------------------
>
Best regards
diff mbox series

Patch

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 2a8304678df9..323c6da9730b 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -58,6 +58,8 @@ 
 #define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD	(1<<17)
 #define S3C64XX_SPI_MODE_BUS_TSZ_WORD		(2<<17)
 #define S3C64XX_SPI_MODE_BUS_TSZ_MASK		(3<<17)
+#define S3C64XX_SPI_MODE_RX_RDY_LVL		GENMASK(16, 11)
+#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT	11
 #define S3C64XX_SPI_MODE_SELF_LOOPBACK		(1<<3)
 #define S3C64XX_SPI_MODE_RXDMA_ON		(1<<2)
 #define S3C64XX_SPI_MODE_TXDMA_ON		(1<<1)
@@ -114,6 +116,8 @@ 
 
 #define S3C64XX_SPI_TRAILCNT		S3C64XX_SPI_MAX_TRAILCNT
 
+#define S3C64XX_SPI_POLLING_SIZE	32
+
 #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
 #define is_polling(x)	(x->cntrlr_info->polling)
 
@@ -552,7 +556,7 @@  static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
 }
 
 static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
-				struct spi_transfer *xfer)
+				struct spi_transfer *xfer, bool use_irq)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long val;
@@ -573,6 +577,12 @@  static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
 	if (RX_FIFO_LVL(status, sdd) < xfer->len)
 		usleep_range(time_us / 2, time_us);
 
+	if (use_irq) {
+		val = msecs_to_jiffies(ms);
+		if (!wait_for_completion_timeout(&sdd->xfer_completion, val))
+			return -EIO;
+	}
+
 	val = msecs_to_loops(ms);
 	do {
 		status = readl(regs + S3C64XX_SPI_STATUS);
@@ -735,10 +745,13 @@  static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	void *rx_buf = NULL;
 	int target_len = 0, origin_len = 0;
 	int use_dma = 0;
+	bool use_irq = false;
 	int status;
 	u32 speed;
 	u8 bpw;
 	unsigned long flags;
+	u32 rdy_lv;
+	u32 val;
 
 	reinit_completion(&sdd->xfer_completion);
 
@@ -759,17 +772,46 @@  static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
 		use_dma = 1;
 
-	} else if (xfer->len > fifo_len) {
+	} else if (xfer->len >= fifo_len) {
 		tx_buf = xfer->tx_buf;
 		rx_buf = xfer->rx_buf;
 		origin_len = xfer->len;
-
 		target_len = xfer->len;
-		if (xfer->len > fifo_len)
-			xfer->len = fifo_len;
+		xfer->len = fifo_len - 1;
 	}
 
 	do {
+		/* transfer size is greater than 32, change to IRQ mode */
+		if (xfer->len > S3C64XX_SPI_POLLING_SIZE)
+			use_irq = true;
+
+		if (use_irq) {
+			reinit_completion(&sdd->xfer_completion);
+
+			rdy_lv = xfer->len;
+			/* Setup RDY_FIFO trigger Level
+			 * RDY_LVL =
+			 * fifo_lvl up to 64 byte -> N bytes
+			 *               128 byte -> RDY_LVL * 2 bytes
+			 *               256 byte -> RDY_LVL * 4 bytes
+			 */
+			if (fifo_len == 128)
+				rdy_lv /= 2;
+			else if (fifo_len == 256)
+				rdy_lv /= 4;
+
+			val = readl(sdd->regs + S3C64XX_SPI_MODE_CFG);
+			val &= ~S3C64XX_SPI_MODE_RX_RDY_LVL;
+			val |= (rdy_lv << S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT);
+			writel(val, sdd->regs + S3C64XX_SPI_MODE_CFG);
+
+			/* Enable FIFO_RDY_EN IRQ */
+			val = readl(sdd->regs + S3C64XX_SPI_INT_EN);
+			writel((val | S3C64XX_SPI_INT_RX_FIFORDY_EN),
+					sdd->regs + S3C64XX_SPI_INT_EN);
+
+		}
+
 		spin_lock_irqsave(&sdd->lock, flags);
 
 		/* Pending only which is to be done */
@@ -791,7 +833,7 @@  static int s3c64xx_spi_transfer_one(struct spi_master *master,
 		if (use_dma)
 			status = s3c64xx_wait_for_dma(sdd, xfer);
 		else
-			status = s3c64xx_wait_for_pio(sdd, xfer);
+			status = s3c64xx_wait_for_pio(sdd, xfer, use_irq);
 
 		if (status) {
 			dev_err(&spi->dev,
@@ -830,8 +872,8 @@  static int s3c64xx_spi_transfer_one(struct spi_master *master,
 			if (xfer->rx_buf)
 				xfer->rx_buf += xfer->len;
 
-			if (target_len > fifo_len)
-				xfer->len = fifo_len;
+			if (target_len >= fifo_len)
+				xfer->len = fifo_len - 1;
 			else
 				xfer->len = target_len;
 		}
@@ -1001,6 +1043,14 @@  static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
 		dev_err(&spi->dev, "TX underrun\n");
 	}
 
+	if (val & S3C64XX_SPI_ST_RX_FIFORDY) {
+		complete(&sdd->xfer_completion);
+		/* No pending clear irq, turn-off INT_EN_RX_FIFO_RDY */
+		val = readl(sdd->regs + S3C64XX_SPI_INT_EN);
+		writel((val & ~S3C64XX_SPI_INT_RX_FIFORDY_EN),
+				sdd->regs + S3C64XX_SPI_INT_EN);
+	}
+
 	/* Clear the pending irq by setting and then clearing it */
 	writel(clr, sdd->regs + S3C64XX_SPI_PENDING_CLR);
 	writel(0, sdd->regs + S3C64XX_SPI_PENDING_CLR);