drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
From: Tom Rix <trix@redhat.com>
Clang static analysis reports this problem
mtk_eth_soc.c:394:7: warning: Branch condition evaluates
to a garbage value
if (err)
^~~
err is not initialized and only conditionally set.
Check err consistently with the rest of mtk_mac_config(),
after even possible setting.
Fixes: 7e538372694b ("net: ethernet: mediatek: Re-add support SGMII")
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index b67b4323cff08..a27e548488584 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -385,14 +385,16 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
0 : mac->id;
/* Setup SGMIISYS with the determined property */
- if (state->interface != PHY_INTERFACE_MODE_SGMII)
+ if (state->interface != PHY_INTERFACE_MODE_SGMII) {
err = mtk_sgmii_setup_mode_force(eth->sgmii, sid,
state);
- else if (phylink_autoneg_inband(mode))
+ if (err)
+ goto init_err;
+ } else if (phylink_autoneg_inband(mode)) {
err = mtk_sgmii_setup_mode_an(eth->sgmii, sid);
-
- if (err)
- goto init_err;
+ if (err)
+ goto init_err;
+ }
regmap_update_bits(eth->ethsys, ETHSYS_SYSCFG0,
SYSCFG0_SGMII_MASK, val);
--
2.26.3
On Sat, 8 Jan 2022 07:50:03 -0800 trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > Clang static analysis reports this problem > mtk_eth_soc.c:394:7: warning: Branch condition evaluates > to a garbage value > if (err) > ^~~ > > err is not initialized and only conditionally set. > Check err consistently with the rest of mtk_mac_config(), > after even possible setting. > > Fixes: 7e538372694b ("net: ethernet: mediatek: Re-add support SGMII") > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index b67b4323cff08..a27e548488584 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -385,14 +385,16 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, > 0 : mac->id; > > /* Setup SGMIISYS with the determined property */ > - if (state->interface != PHY_INTERFACE_MODE_SGMII) > + if (state->interface != PHY_INTERFACE_MODE_SGMII) { > err = mtk_sgmii_setup_mode_force(eth->sgmii, sid, > state); > - else if (phylink_autoneg_inband(mode)) > + if (err) > + goto init_err; > + } else if (phylink_autoneg_inband(mode)) { > err = mtk_sgmii_setup_mode_an(eth->sgmii, sid); > - > - if (err) > - goto init_err; > + if (err) > + goto init_err; > + } > > regmap_update_bits(eth->ethsys, ETHSYS_SYSCFG0, > SYSCFG0_SGMII_MASK, val); Why not init err to 0 before the if or add an else err = 0; branch?
On 1/9/22 4:43 PM, Jakub Kicinski wrote: > On Sat, 8 Jan 2022 07:50:03 -0800 trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> Clang static analysis reports this problem >> mtk_eth_soc.c:394:7: warning: Branch condition evaluates >> to a garbage value >> if (err) >> ^~~ >> >> err is not initialized and only conditionally set. >> Check err consistently with the rest of mtk_mac_config(), >> after even possible setting. >> >> Fixes: 7e538372694b ("net: ethernet: mediatek: Re-add support SGMII") >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c >> index b67b4323cff08..a27e548488584 100644 >> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c >> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c >> @@ -385,14 +385,16 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, >> 0 : mac->id; >> >> /* Setup SGMIISYS with the determined property */ >> - if (state->interface != PHY_INTERFACE_MODE_SGMII) >> + if (state->interface != PHY_INTERFACE_MODE_SGMII) { >> err = mtk_sgmii_setup_mode_force(eth->sgmii, sid, >> state); >> - else if (phylink_autoneg_inband(mode)) >> + if (err) >> + goto init_err; >> + } else if (phylink_autoneg_inband(mode)) { >> err = mtk_sgmii_setup_mode_an(eth->sgmii, sid); >> - >> - if (err) >> - goto init_err; >> + if (err) >> + goto init_err; >> + } >> >> regmap_update_bits(eth->ethsys, ETHSYS_SYSCFG0, >> SYSCFG0_SGMII_MASK, val); > Why not init err to 0 before the if or add an else err = 0; branch? This is the way I would have preferred to do it but the function's existing error handling does it this way. I'll respin the patch. Tom >
© 2016 - 2024 Red Hat, Inc.