On Thu, Apr 16, 2020 at 1:24 PM Nathan Chancellor <natechancellor@xxxxxxxxx> wrote: > > On Thu, Apr 16, 2020 at 01:16:27PM -0700, Nick Desaulniers wrote: > > On Thu, Apr 16, 2020 at 11:24 AM Nathan Chancellor > > <natechancellor@xxxxxxxxx> 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 > > 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. 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 <ndesaulniers@xxxxxxxxxx> -- Thanks, ~Nick Desaulniers
