[PATCH v2 3/3] spi: fsl-qspi: Use reinit_completion() for repeated operations

Felix Gu posted 3 patches 1 month, 1 week ago
[PATCH v2 3/3] spi: fsl-qspi: Use reinit_completion() for repeated operations
Posted by Felix Gu 1 month, 1 week ago
The driver currently calls init_completion() during every spi_mem_op.
Tchnically it may work, but it's not the recommended pattern.

According to the kernel documentation: Calling init_completion() on
the same completion object twice is most likely a bug as it
re-initializes the queue to an empty queue and enqueued tasks could
get "lost" - use reinit_completion() in that case, but be aware of
other races.

So moves the initial initialization to probe function and uses
reinit_completion() for subsequent operations.

Fixes: 84d043185dbe ("spi: Add a driver for the Freescale/NXP QuadSPI controller")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/spi/spi-fsl-qspi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index a223b4bc6e63..57358851029b 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -633,7 +633,7 @@ static int fsl_qspi_do_op(struct fsl_qspi *q, const struct spi_mem_op *op)
 	void __iomem *base = q->iobase;
 	int err = 0;
 
-	init_completion(&q->c);
+	reinit_completion(&q->c);
 
 	/*
 	 * Always start the sequence at the same index since we update
@@ -965,6 +965,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	init_completion(&q->c);
 	ret = devm_request_irq(dev, ret,
 			fsl_qspi_irq_handler, 0, pdev->name, q);
 	if (ret) {

-- 
2.43.0
Re: [PATCH v2 3/3] spi: fsl-qspi: Use reinit_completion() for repeated operations
Posted by Frank Li 1 month, 1 week ago
From: Frank Li (AI-BOT) <frank.li@nxp.com>

Commit message has a typo:

> Tchnically it may work, but it's not the recommended pattern.

Should be "Technically".

Also, the placement of init_completion() in the probe function should come
before devm_request_irq(), since the IRQ handler may reference the
completion object. The current placement is correct, but consider adding a
comment explaining why the initialization must happen before IRQ
registration.

Otherwise the patch looks good.

AI bot review and may be useless.