The ret variable in mtk_hdmi_pll_calc() was used unitialized as reported
by the kernel test robot.
Fix the issue by removing the variable altogether and testing out the
return value of mtk_hdmi_pll_set_hw()
Fixes: 45810d486bb44 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
---
drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c
index abfc077fb0a8..e10da6c4147e 100644
--- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c
+++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c
@@ -213,7 +213,7 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw,
u64 tmds_clk, pixel_clk, da_hdmitx21_ref_ck, ns_hdmipll_ck, pcw;
u8 txpredivs[4] = { 2, 4, 6, 12 };
u32 fbkdiv_low;
- int i, ret;
+ int i;
pixel_clk = rate;
tmds_clk = pixel_clk;
@@ -292,10 +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,
+ if (mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low,
PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv,
- txposdiv, digital_div);
- if (ret)
+ txposdiv, digital_div))
return -EINVAL;
return 0;
--
2.39.2
Il 13/04/23 14:46, Guillaume Ranquet ha scritto: > The ret variable in mtk_hdmi_pll_calc() was used unitialized as reported > by the kernel test robot. > > Fix the issue by removing the variable altogether and testing out the > return value of mtk_hdmi_pll_set_hw() > > Fixes: 45810d486bb44 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > index abfc077fb0a8..e10da6c4147e 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > @@ -213,7 +213,7 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw, > u64 tmds_clk, pixel_clk, da_hdmitx21_ref_ck, ns_hdmipll_ck, pcw; > u8 txpredivs[4] = { 2, 4, 6, 12 }; > u32 fbkdiv_low; > - int i, ret; > + int i; > > pixel_clk = rate; > tmds_clk = pixel_clk; > @@ -292,10 +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, > + if (mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > - txposdiv, digital_div); > - if (ret) > + txposdiv, digital_div)) > return -EINVAL; > I don't get why we're returning -EINVAL unconditionally in the first place, here. Function mtk_hdmi_pll_set_hw() should return zero or a negative error number: in that case, the previous *intention* was fine, so this should be ret = mtk_hdmi_pll_set_hw(....) if (ret) return ret; return 0; Regards, Angelo
On Fri, 14 Apr 2023 12:31, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: >Il 13/04/23 14:46, Guillaume Ranquet ha scritto: >> The ret variable in mtk_hdmi_pll_calc() was used unitialized as reported >> by the kernel test robot. >> >> Fix the issue by removing the variable altogether and testing out the >> return value of mtk_hdmi_pll_set_hw() >> >> Fixes: 45810d486bb44 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> >> --- >> drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> index abfc077fb0a8..e10da6c4147e 100644 >> --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c >> @@ -213,7 +213,7 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw, >> u64 tmds_clk, pixel_clk, da_hdmitx21_ref_ck, ns_hdmipll_ck, pcw; >> u8 txpredivs[4] = { 2, 4, 6, 12 }; >> u32 fbkdiv_low; >> - int i, ret; >> + int i; >> >> pixel_clk = rate; >> tmds_clk = pixel_clk; >> @@ -292,10 +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, >> + if (mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, >> PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, >> - txposdiv, digital_div); >> - if (ret) >> + txposdiv, digital_div)) >> return -EINVAL; >> > >I don't get why we're returning -EINVAL unconditionally in the first place, here. > >Function mtk_hdmi_pll_set_hw() should return zero or a negative error number: in >that case, the previous *intention* was fine, so this should be > Hi Angelo, I was maybe a bit too quick on fixing this that way. Anyway it doesn't change a thing as mtk_hdmi_pll_set_hw() eitheir returns 0 or -EINVAL. But I agree that the logic is dubious and propagating the return value is the right thing to do. I see that Arnd and Tom posted versions that you might prefer: https://lore.kernel.org/linux-phy/20230414075842.4006164-1-arnd@kernel.org/ https://lore.kernel.org/linux-phy/20230414122253.3171524-1-trix@redhat.com/ Thx, Guillaume. > ret = mtk_hdmi_pll_set_hw(....) > if (ret) > return ret; > > return 0; > > >Regards, >Angelo
© 2016 - 2024 Red Hat, Inc.