[PATCH 11/31] staging: mt7621-mmc: Remove multiple assignments
Christian Luetke
christian at lkamp.de
Fri Apr 20 23:27:09 UTC 2018
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 at lkamp.de>
>> ---
>> 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
More information about the devel
mailing list