Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: ethernet: mtk_eth_soc: fix error checking in mtk_mac_config()
@ 2022-01-08 15:50 trix
2022-01-10 0:43 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: trix @ 2022-01-08 15:50 UTC (permalink / raw)
To: nbd, john, sean.wang, Mark-MC.Lee, davem, kuba, matthias.bgg,
linux, nathan, ndesaulniers, opensource
Cc: netdev, linux-arm-kernel, linux-mediatek, linux-kernel, llvm, Tom Rix
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] net: ethernet: mtk_eth_soc: fix error checking in mtk_mac_config()
2022-01-08 15:50 [PATCH] net: ethernet: mtk_eth_soc: fix error checking in mtk_mac_config() trix
@ 2022-01-10 0:43 ` Jakub Kicinski
2022-01-10 13:43 ` Tom Rix
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2022-01-10 0:43 UTC (permalink / raw)
To: trix
Cc: nbd, john, sean.wang, Mark-MC.Lee, davem, matthias.bgg, linux,
nathan, ndesaulniers, opensource, netdev, linux-arm-kernel,
linux-mediatek, linux-kernel, llvm
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?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] net: ethernet: mtk_eth_soc: fix error checking in mtk_mac_config()
2022-01-10 0:43 ` Jakub Kicinski
@ 2022-01-10 13:43 ` Tom Rix
0 siblings, 0 replies; 3+ messages in thread
From: Tom Rix @ 2022-01-10 13:43 UTC (permalink / raw)
To: Jakub Kicinski
Cc: nbd, john, sean.wang, Mark-MC.Lee, davem, matthias.bgg, linux,
nathan, ndesaulniers, opensource, netdev, linux-arm-kernel,
linux-mediatek, linux-kernel, llvm
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-01-10 13:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-08 15:50 [PATCH] net: ethernet: mtk_eth_soc: fix error checking in mtk_mac_config() trix
2022-01-10 0:43 ` Jakub Kicinski
2022-01-10 13:43 ` Tom Rix
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).