[PATCH] mmc: sdhci-of-arasan: Remove uninitialized ret variables

[Nathan Chancellor]

Clang warns:

drivers/mmc/host/sdhci-of-arasan.c:784:9: warning: variable 'ret' is
uninitialized when used here [-Wuninitialized]
return ret;
^~~
drivers/mmc/host/sdhci-of-arasan.c:738:9: note: initialize the variable
'ret' to silence this warning
int ret;
^
= 0
drivers/mmc/host/sdhci-of-arasan.c:860:9: warning: variable 'ret' is
uninitialized when used here [-Wuninitialized]
return ret;
^~~
drivers/mmc/host/sdhci-of-arasan.c:810:9: note: initialize the variable
'ret' to silence this warning
int ret;
^
= 0
2 warnings generated.

This looks like a copy paste error. Neither function has handling that
needs ret so just remove it and return 0 directly.

Fixes: f73e66a36772 ("sdhci: arasan: Add support for Versal Tap Delays")
Link: https://github.com/ClangBuiltLinux/linux/issues/996
Reported-by: kernelci.org bot <b...@kernelci.org>
Signed-off-by: Nathan Chancellor <natecha...@gmail.com>
---
drivers/mmc/host/sdhci-of-arasan.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 16e26c217a77..18bf0e76b1eb 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -735,7 +735,6 @@ static int sdhci_versal_sdcardclk_set_phase(struct clk_hw *hw, int degrees)
container_of(clk_data, struct sdhci_arasan_data, clk_data);
struct sdhci_host *host = sdhci_arasan->host;
u8 tap_delay, tap_max = 0;
- int ret;

/*
* This is applicable for SDHCI_SPEC_300 and above
@@ -781,7 +780,7 @@ static int sdhci_versal_sdcardclk_set_phase(struct clk_hw *hw, int degrees)
sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER);
}

- return ret;
+ return 0;
}

static const struct clk_ops versal_sdcardclk_ops = {
@@ -807,7 +806,6 @@ static int sdhci_versal_sampleclk_set_phase(struct clk_hw *hw, int degrees)
container_of(clk_data, struct sdhci_arasan_data, clk_data);
struct sdhci_host *host = sdhci_arasan->host;
u8 tap_delay, tap_max = 0;
- int ret;

/*
* This is applicable for SDHCI_SPEC_300 and above
@@ -857,7 +855,7 @@ static int sdhci_versal_sampleclk_set_phase(struct clk_hw *hw, int degrees)
sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
}

- return ret;
+ return 0;
}

static const struct clk_ops versal_sampleclk_ops = {

base-commit: a3ca59b9af21e68069555ffff1ad89bd2a7c40fc
--
2.26.1

[Nick Desaulniers]

On Thu, Apr 16, 2020 at 11:24 AM Nathan Chancellor
<natecha...@gmail.com> wrote:
>
> Clang warns:
>
> drivers/mmc/host/sdhci-of-arasan.c:784:9: warning: variable 'ret' is
> uninitialized when used here [-Wuninitialized]
> return ret;
> ^~~
> drivers/mmc/host/sdhci-of-arasan.c:738:9: note: initialize the variable
> 'ret' to silence this warning
> int ret;
> ^
> = 0
> drivers/mmc/host/sdhci-of-arasan.c:860:9: warning: variable 'ret' is
> uninitialized when used here [-Wuninitialized]
> return ret;
> ^~~
> drivers/mmc/host/sdhci-of-arasan.c:810:9: note: initialize the variable
> 'ret' to silence this warning
> int ret;
> ^
> = 0
> 2 warnings generated.
>
> This looks like a copy paste error. Neither function has handling that
> needs ret so just remove it and return 0 directly.

Forgive me for not taking the time to look into this more carefully,
but just a thought:

Having functions always return a single integer literal as opposed to
having a `void` return type in their function signature is a code
smell. Did you consider the call sites of these functions to see if
they do anything with the return value? I understand it may not be
worthwhile/possible if these functions fulfil an interface that
requires the int return type function signature. (It's also probably
faster for me to just look rather than type this all out, but I saw no
mention of this consideration in the commit message or patch, so
wanted to check that it had been performed).

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200416182402.16858-1-natechancellor%40gmail.com.



--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

Which is the case. These functions are passed to 'struct clk_ops', which
defines the set_phase member as

int (*set_phase)(struct clk_hw *hw, int degrees);

so we cannot just change the return to void since there are other
set_phase functions that do set a return value other than zero.

> faster for me to just look rather than type this all out, but I saw no
> mention of this consideration in the commit message or patch, so
> wanted to check that it had been performed).

Yeah, I should have probably mentioned that. I can do so if the
maintainers feel it worthwhile.

Cheers,
Nathan

[Nick Desaulniers]

No worries, I should hold my comments until I can give a more thorough
review, which I've now done. LGTM and thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesau...@google.com>


--
Thanks,
~Nick Desaulniers

[Ulf Hansson]

Applied for next, thanks!

Kind regards
Uffe