diff mbox series

spi: spi-zynq-qspi: use wait_for_completion_timeout to make zynq_qspi_exec_mem_op not interruptible

Message ID 20210730031753.1317917-1-quanyang.wang@windriver.com (mailing list archive)
State New, archived
Headers show
Series spi: spi-zynq-qspi: use wait_for_completion_timeout to make zynq_qspi_exec_mem_op not interruptible | expand

Commit Message

Quanyang Wang July 30, 2021, 3:17 a.m. UTC
From: Quanyang Wang <quanyang.wang@windriver.com>

The function wait_for_completion_interruptible_timeout will return
-ERESTARTSYS immediately when receiving SIGKILL signal which is sent
by "jffs2_gcd_mtd" during umounting jffs2. This will break the SPI memory
operation because the data transmitting may begin before the command or
address transmitting completes. Use wait_for_completion_timeout to prevent
the process from being interruptible.

Fixes: 67dca5e580f1 ("spi: spi-mem: Add support for Zynq QSPI controller")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
---
 drivers/spi/spi-zynq-qspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Quanyang Wang Aug. 6, 2021, 12:41 a.m. UTC | #1
ping.

On 7/30/21 11:17 AM, quanyang.wang@windriver.com wrote:
> From: Quanyang Wang <quanyang.wang@windriver.com>
> 
> The function wait_for_completion_interruptible_timeout will return
> -ERESTARTSYS immediately when receiving SIGKILL signal which is sent
> by "jffs2_gcd_mtd" during umounting jffs2. This will break the SPI memory
> operation because the data transmitting may begin before the command or
> address transmitting completes. Use wait_for_completion_timeout to prevent
> the process from being interruptible.
> 
> Fixes: 67dca5e580f1 ("spi: spi-mem: Add support for Zynq QSPI controller")
> Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
> ---
>   drivers/spi/spi-zynq-qspi.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
> index 9262c6418463..cfa222c9bd5e 100644
> --- a/drivers/spi/spi-zynq-qspi.c
> +++ b/drivers/spi/spi-zynq-qspi.c
> @@ -545,7 +545,7 @@ static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
>   		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
>   		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
>   				ZYNQ_QSPI_IXR_RXTX_MASK);
> -		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
> +		if (!wait_for_completion_timeout(&xqspi->data_completion,
>   							       msecs_to_jiffies(1000)))
>   			err = -ETIMEDOUT;
>   	}
> @@ -563,7 +563,7 @@ static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
>   		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
>   		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
>   				ZYNQ_QSPI_IXR_RXTX_MASK);
> -		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
> +		if (!wait_for_completion_timeout(&xqspi->data_completion,
>   							       msecs_to_jiffies(1000)))
>   			err = -ETIMEDOUT;
>   	}
> @@ -579,7 +579,7 @@ static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
>   		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
>   		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
>   				ZYNQ_QSPI_IXR_RXTX_MASK);
> -		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
> +		if (!wait_for_completion_timeout(&xqspi->data_completion,
>   							       msecs_to_jiffies(1000)))
>   			err = -ETIMEDOUT;
>   
> @@ -603,7 +603,7 @@ static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
>   		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
>   		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
>   				ZYNQ_QSPI_IXR_RXTX_MASK);
> -		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
> +		if (!wait_for_completion_timeout(&xqspi->data_completion,
>   							       msecs_to_jiffies(1000)))
>   			err = -ETIMEDOUT;
>   	}
>
Mark Brown Aug. 6, 2021, 12:47 a.m. UTC | #2
On Fri, Aug 06, 2021 at 08:41:41AM +0800, quanyang.wang wrote:
> ping.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.
diff mbox series

Patch

diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
index 9262c6418463..cfa222c9bd5e 100644
--- a/drivers/spi/spi-zynq-qspi.c
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -545,7 +545,7 @@  static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
 		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
 		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
 				ZYNQ_QSPI_IXR_RXTX_MASK);
-		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
+		if (!wait_for_completion_timeout(&xqspi->data_completion,
 							       msecs_to_jiffies(1000)))
 			err = -ETIMEDOUT;
 	}
@@ -563,7 +563,7 @@  static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
 		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
 		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
 				ZYNQ_QSPI_IXR_RXTX_MASK);
-		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
+		if (!wait_for_completion_timeout(&xqspi->data_completion,
 							       msecs_to_jiffies(1000)))
 			err = -ETIMEDOUT;
 	}
@@ -579,7 +579,7 @@  static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
 		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
 		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
 				ZYNQ_QSPI_IXR_RXTX_MASK);
-		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
+		if (!wait_for_completion_timeout(&xqspi->data_completion,
 							       msecs_to_jiffies(1000)))
 			err = -ETIMEDOUT;
 
@@ -603,7 +603,7 @@  static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
 		zynq_qspi_write_op(xqspi, ZYNQ_QSPI_FIFO_DEPTH, true);
 		zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
 				ZYNQ_QSPI_IXR_RXTX_MASK);
-		if (!wait_for_completion_interruptible_timeout(&xqspi->data_completion,
+		if (!wait_for_completion_timeout(&xqspi->data_completion,
 							       msecs_to_jiffies(1000)))
 			err = -ETIMEDOUT;
 	}