| Message ID | 20230414122253.3171524-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | phy: mediatek: fix returning garbage | expand |
On 14/04/2023 14:22, Tom Rix wrote: > clang reports > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable > 'ret' is uninitialized when used here [-Werror,-Wuninitialized] > if (ret) > ^~~ > ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. > > Fixes: 45810d486bb4 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") > Signed-off-by: Tom Rix <trix@redhat.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > index abfc077fb0a8..c63294e451d6 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > @@ -292,9 +292,9 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw, > if (!(digital_div <= 32 && digital_div >= 1)) > return -EINVAL; > > - mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > - PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > - txposdiv, digital_div); > + ret = mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > + PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > + txposdiv, digital_div); > if (ret) > return -EINVAL; >
On 14/04/2023 17:43, Matthias Brugger wrote: > > > On 14/04/2023 14:22, Tom Rix wrote: >> clang reports >> drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable >> 'ret' is uninitialized when used here [-Werror,-Wuninitialized] >> if (ret) >> ^~~ >> ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. >> >> Fixes: 45810d486bb4 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") >> Signed-off-by: Tom Rix <trix@redhat.com> > > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > Please also add Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> see CA+G9fYu4o0-ZKSthi7kdCjz_kFazZS-rn17Z2NPz3=1Oayr9cw@mail.gmail.com Regards, Matthias >> --- >> drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> index abfc077fb0a8..c63294e451d6 100644 >> --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> @@ -292,9 +292,9 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy >> *hdmi_phy, struct clk_hw *hw, >> if (!(digital_div <= 32 && digital_div >= 1)) >> return -EINVAL; >> - mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, >> - PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, >> - txposdiv, digital_div); >> + ret = mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, >> + PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, >> + txposdiv, digital_div); >> if (ret) >> return -EINVAL;
On Fri, Apr 14, 2023 at 08:22:53AM -0400, Tom Rix wrote: > clang reports > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable > 'ret' is uninitialized when used here [-Werror,-Wuninitialized] > if (ret) > ^~~ > ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. > > Fixes: 45810d486bb4 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") > Signed-off-by: Tom Rix <trix@redhat.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Angelo brought up a good point on another patch to fix this issue that we should probably be returning ret instead of unconditionally returning -EINVAL but it sounds like it does not really matter at the moment since mtk_hdmi_pll_set_hw() can only return -EINVAL or 0. https://lore.kernel.org/ada769e0-0141-634f-3753-eb3a50f0eee3@collabora.com/ > --- > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > index abfc077fb0a8..c63294e451d6 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > @@ -292,9 +292,9 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw, > if (!(digital_div <= 32 && digital_div >= 1)) > return -EINVAL; > > - mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > - PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > - txposdiv, digital_div); > + ret = mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > + PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > + txposdiv, digital_div); > if (ret) > return -EINVAL; > > -- > 2.27.0 >
Il 14/04/23 14:22, Tom Rix ha scritto: > clang reports > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable > 'ret' is uninitialized when used here [-Werror,-Wuninitialized] > if (ret) > ^~~ > ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. > > Fixes: 45810d486bb4 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") > Signed-off-by: Tom Rix <trix@redhat.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On 14-04-23, 08:22, Tom Rix wrote: > clang reports > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable > 'ret' is uninitialized when used here [-Werror,-Wuninitialized] > if (ret) > ^~~ > ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. I have applied "phy: mediatek: hdmi: mt8195: fix uninitialized variable usage in pll_calc"
On 05/05/2023 11:28, Vinod Koul wrote: > On 14-04-23, 08:22, Tom Rix wrote: >> clang reports >> drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable >> 'ret' is uninitialized when used here [-Werror,-Wuninitialized] >> if (ret) >> ^~~ >> ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. > > I have applied "phy: mediatek: hdmi: mt8195: fix uninitialized variable > usage in pll_calc" Thanks Vinod, that was on my list for today as well. I was a bit puzzled because you took the patch that added it, but I wasn't sure if you would take the fix. How do you want to handle things like this in the future? Regards, Matthias
diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c index abfc077fb0a8..c63294e451d6 100644 --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c @@ -292,9 +292,9 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw, if (!(digital_div <= 32 && digital_div >= 1)) return -EINVAL; - mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, - PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, - txposdiv, digital_div); + ret = mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, + PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, + txposdiv, digital_div); if (ret) return -EINVAL;
clang reports drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable 'ret' is uninitialized when used here [-Werror,-Wuninitialized] if (ret) ^~~ ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. Fixes: 45810d486bb4 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") Signed-off-by: Tom Rix <trix@redhat.com> --- drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)