Re: [PATCH 11/31] staging: mt7621-mmc: Remove multiple assignments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 21.04.2018 00:22, schrieb NeilBrown:
On Wed, Apr 18 2018, Christian Lütke-Stetzkamp wrote:

Fix checkpatch: multiple assignments should be avoided, to improve
readability.

Signed-off-by: Christian Lütke-Stetzkamp <christian@xxxxxxxx>
---
 drivers/staging/mt7621-mmc/sd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 3871602b4651..357c10551773 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -1473,11 +1473,12 @@ static int msdc_do_request(struct mmc_host *mmc, struct mmc_request *mrq)

 		/* deside the transfer mode */
 		if (drv_mode[host->id] == MODE_PIO)
-			host->dma_xfer = dma = 0;
+			host->dma_xfer = 0;
 		else if (drv_mode[host->id] == MODE_DMA)
-			host->dma_xfer = dma = 1;
+			host->dma_xfer = 1;
 		else if (drv_mode[host->id] == MODE_SIZE_DEP)
- host->dma_xfer = dma = ((host->xfer_size >= dma_size[host->id]) ? 1 : 0); + host->dma_xfer = ((host->xfer_size >= dma_size[host->id]) ? 1 : 0);
+		dma = host->dma_xfer;

You've changed behaviour here.
Previously, if none of the 3 conditions were true, dma would have the
value of 0 that it was initialized to.
No it will take the value of host->dma_xfer from the previous transfer.
I doubt that is correct.

I would change the if branches to assign to dma, not to host->dma_xfer.
Then "host->dma_xfer = dma;". This isn't *quite* what the original code
does, but I think it is close enough.

But drv_mode has the type msdc_mode, that is an enum that has only the
elements MODE_PIO, MODE_DMA and MODE_SIZE_DEP. So the original code is not optimal, maybe the last else if should be an else. Shall I change the patch to respect that? Or should I change it to a switch case? (Also this code will be removed in the next series, as you suggested, what I already thought of -
removing the non DMA mode. )

Thanks for the review.

Regards,
Christian
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux