* [PATCH 1/2] mtd: nand: mxc_nand: use 'time_left' variable with wait_for_completion_timeout()
2024-06-04 21:29 [PATCH 0/2] mtd: use 'time_left' instead of 'timeout' with wait_*() functions Wolfram Sang
@ 2024-06-04 21:29 ` Wolfram Sang
2024-07-01 12:00 ` Miquel Raynal
2024-06-04 21:29 ` [PATCH 2/2] mtd: rawnand: intel: " Wolfram Sang
2024-06-05 7:43 ` [PATCH 0/2] mtd: use 'time_left' instead of 'timeout' with wait_*() functions Miquel Raynal
2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2024-06-04 21:29 UTC (permalink / raw)
To: linux-kernel
Cc: Wolfram Sang, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd
There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:
timeout = wait_for_completion_timeout(...)
if (!timeout) return -ETIMEDOUT;
with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mtd/nand/raw/mxc_nand.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
index 9d50f41b3d5a..e22fbea970d1 100644
--- a/drivers/mtd/nand/raw/mxc_nand.c
+++ b/drivers/mtd/nand/raw/mxc_nand.c
@@ -457,14 +457,14 @@ static int wait_op_done(struct mxc_nand_host *host, int useirq)
return 0;
if (useirq) {
- unsigned long timeout;
+ unsigned long time_left;
reinit_completion(&host->op_completion);
irq_control(host, 1);
- timeout = wait_for_completion_timeout(&host->op_completion, HZ);
- if (!timeout && !host->devtype_data->check_int(host)) {
+ time_left = wait_for_completion_timeout(&host->op_completion, HZ);
+ if (!time_left && !host->devtype_data->check_int(host)) {
dev_dbg(host->dev, "timeout waiting for irq\n");
ret = -ETIMEDOUT;
}
--
2.43.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] mtd: rawnand: intel: use 'time_left' variable with wait_for_completion_timeout()
2024-06-04 21:29 [PATCH 0/2] mtd: use 'time_left' instead of 'timeout' with wait_*() functions Wolfram Sang
2024-06-04 21:29 ` [PATCH 1/2] mtd: nand: mxc_nand: use 'time_left' variable with wait_for_completion_timeout() Wolfram Sang
@ 2024-06-04 21:29 ` Wolfram Sang
2024-07-01 12:00 ` Miquel Raynal
2024-06-05 7:43 ` [PATCH 0/2] mtd: use 'time_left' instead of 'timeout' with wait_*() functions Miquel Raynal
2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2024-06-04 21:29 UTC (permalink / raw)
To: linux-kernel
Cc: Wolfram Sang, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd
There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:
timeout = wait_for_completion_timeout(...)
if (!timeout) return -ETIMEDOUT;
with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.
Fix to the proper variable type 'unsigned long' while here.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mtd/nand/raw/intel-nand-controller.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
index f0ad2308f6d5..78174c463b36 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -295,7 +295,7 @@ static int ebu_dma_start(struct ebu_nand_controller *ebu_host, u32 dir,
unsigned long flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
dma_addr_t buf_dma;
int ret;
- u32 timeout;
+ unsigned long time_left;
if (dir == DMA_DEV_TO_MEM) {
chan = ebu_host->dma_rx;
@@ -335,8 +335,8 @@ static int ebu_dma_start(struct ebu_nand_controller *ebu_host, u32 dir,
dma_async_issue_pending(chan);
/* Wait DMA to finish the data transfer.*/
- timeout = wait_for_completion_timeout(dma_completion, msecs_to_jiffies(1000));
- if (!timeout) {
+ time_left = wait_for_completion_timeout(dma_completion, msecs_to_jiffies(1000));
+ if (!time_left) {
dev_err(ebu_host->dev, "I/O Error in DMA RX (status %d)\n",
dmaengine_tx_status(chan, cookie, NULL));
dmaengine_terminate_sync(chan);
--
2.43.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] mtd: use 'time_left' instead of 'timeout' with wait_*() functions
2024-06-04 21:29 [PATCH 0/2] mtd: use 'time_left' instead of 'timeout' with wait_*() functions Wolfram Sang
2024-06-04 21:29 ` [PATCH 1/2] mtd: nand: mxc_nand: use 'time_left' variable with wait_for_completion_timeout() Wolfram Sang
2024-06-04 21:29 ` [PATCH 2/2] mtd: rawnand: intel: " Wolfram Sang
@ 2024-06-05 7:43 ` Miquel Raynal
2 siblings, 0 replies; 6+ messages in thread
From: Miquel Raynal @ 2024-06-05 7:43 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, linux-mtd, Richard Weinberger, Vignesh Raghavendra
Hi Wolfram,
wsa+renesas@sang-engineering.com wrote on Tue, 4 Jun 2024 23:29:19
+0200:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_*() functions causing patterns like:
>
> timeout = wait_for_completion_timeout(...)
> if (!timeout) return -ETIMEDOUT;
>
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> obvious and self explaining.
Good idea. The waiting functions are generally confusing because the
semantics of the return values are different, so this is a good first
step, I'll take them through mtd-next soon.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread