lkml.org 
[lkml]   [2018]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] mmc: sdhci-cadence: fix logically and structurally dead code
From
Date
I got it.

I'll send v2 shortly.

Thanks for the feedback, Masahiro.

--
Gustavo

On 04/19/2018 10:42 AM, Masahiro Yamada wrote:
> Hi.
>
>
> 2018-04-19 22:53 GMT+09:00 Gustavo A. R. Silva <gustavo@embeddedor.com>:
>> Currently, the code block inside the for loop will never execute
>> more than once, because the function returns inmediately after
>> the first iteration, hence the execution of the code at the second
>> iteration is structurally dead and, code at line 281: return 0; is
>> never reached.
>>
>> Based on the code comments, it seems that the actual intention is
>> to execute the code inside the for loop twice instead of once.
>
> Thanks for the report.
>
>> So, fix this issue by removing the return statement inside the for
>> loop and replace the "return 0" with "return ret".
>>
>> Addresses-Coverity-ID: 1468009 ("Logically dead code")
>> Addresses-Coverity-ID: 1468002 ("Structurally dead code")
>> Fixes: 213fae74318b ("mmc: sdhci-cadence: send tune request twice to
>> work around errata")
>
>
> Probably, this Fixes tag will dangle.
>
> Ulf usually repeats git-rebase to build-up his pull-request.
>
> The addressed commit was already rebased,
> and its commit ID will change a few more times
> since it is now -rc1.
>
>
> A clean solution would be, to squash a fix-up into the original patch.
> (This patch is not what I want, though.)
>
> If you want to claim contribution in a separate patch,
> please rewrite the code as I suggested,
> and drop the Fixes tag.
>
>
>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> drivers/mmc/host/sdhci-cadence.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
>> index bc30d16..facbad8 100644
>> --- a/drivers/mmc/host/sdhci-cadence.c
>> +++ b/drivers/mmc/host/sdhci-cadence.c
>> @@ -275,10 +275,9 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>> !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
>> 0, 1);
>>
>> - return ret;
>> }
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static int sdhci_cdns_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> --
>
> No.
> I want to confirm that the operation succeeds twice.
>
> Your code hides any error in the first loop.
>
>
>
> My intention is like follows:
>
>
>
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -275,10 +275,9 @@ static int sdhci_cdns_set_tune_val(struct
> sdhci_host *host, unsigned int val)
> !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
> 0, 1);
> -
> - return ret;
> + if (ret)
> + return ret;
> }
>
> return 0;
> }
>
> static int sdhci_cdns_execute_tuning(struct mmc_host *mmc, u32 opcode)
>
>
>

\
 
 \ /
  Last update: 2018-04-19 17:48    [W:0.043 / U:1.876 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site