2023-08-13 21:30:32

by Artem Chernyshev

Subject: [PATCH] brcm80211: brcmsmac: phy_cmn: Remove unreachable code

Since wlc_phy_txpwr_srom_read_nphy() in wlc_phy_attach_nphy()
can not return false it's impossible to get true value in this
if statement

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Artem Chernyshev <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c
index ccc621b8ed9f..07f83ff5a54a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c
@@ -551,8 +551,7 @@ wlc_phy_attach(struct shared_phy *sh, struct bcma_device *d11core,
if (!pi->phycal_timer)
goto err;

- if (!wlc_phy_attach_nphy(pi))
- goto err;
+ wlc_phy_attach_nphy(pi);

} else if (ISLCNPHY(pi)) {
if (!wlc_phy_attach_lcnphy(pi))
--
2.37.3

2023-08-14 07:42:18

by Jonas Gorski

Subject: Re: [PATCH] brcm80211: brcmsmac: phy_cmn: Remove unreachable code

On Sun, 13 Aug 2023 at 23:31, Artem Chernyshev
<[email protected]> wrote:
>
> Since wlc_phy_txpwr_srom_read_nphy() in wlc_phy_attach_nphy()
> can not return false it's impossible to get true value in this
> if statement

If they can only return true, then maybe their return types should be
changed to void as well? No point in returning a value when the only
caller is ignoring it.

>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Artem Chernyshev <[email protected]>

Regards,
Jonas

2023-08-14 07:43:51

by Artem Chernyshev

Subject: Re: [PATCH] brcm80211: brcmsmac: phy_cmn: Remove unreachable code

On Mon, Aug 14, 2023 at 09:19:14AM +0200, Jonas Gorski wrote:
> On Sun, 13 Aug 2023 at 23:31, Artem Chernyshev
> <[email protected]> wrote:
> >
> > Since wlc_phy_txpwr_srom_read_nphy() in wlc_phy_attach_nphy()
> > can not return false it's impossible to get true value in this
> > if statement
>
> If they can only return true, then maybe their return types should be
> changed to void as well? No point in returning a value when the only
> caller is ignoring it.
>
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >
> > Signed-off-by: Artem Chernyshev <[email protected]>
>
> Regards,
> Jonas

True that. I'll make those changes in v2.

Thank you,
Artem

2023-08-14 10:09:40

by Artem Chernyshev

Subject: [PATCH v2] brcm80211: brcmsmac: phy: Remove unreachable code

wlc_phy_txpwr_srom_read_nphy() in wlc_phy_attach_nphy() can not
return false, so it's impossible to get true value in this
if-statement. Also change those functions return types to void
since no one using it.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Artem Chernyshev <[email protected]>
---
V1->V2 Change return types to void

.../broadcom/brcm80211/brcmsmac/phy/phy_cmn.c | 3 +--
.../broadcom/brcm80211/brcmsmac/phy/phy_int.h | 2 +-
.../wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c | 11 +++--------
3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c
index ccc621b8ed9f..07f83ff5a54a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_cmn.c
@@ -551,8 +551,7 @@ wlc_phy_attach(struct shared_phy *sh, struct bcma_device *d11core,
if (!pi->phycal_timer)
goto err;

- if (!wlc_phy_attach_nphy(pi))
- goto err;
+ wlc_phy_attach_nphy(pi);

} else if (ISLCNPHY(pi)) {
if (!wlc_phy_attach_lcnphy(pi))
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h
index 8668fa5558a2..70a9ec050717 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h
@@ -941,7 +941,7 @@ void wlc_phy_papd_decode_epsilon(u32 epsilon, s32 *eps_real, s32 *eps_imag);
void wlc_phy_cal_perical_mphase_reset(struct brcms_phy *pi);
void wlc_phy_cal_perical_mphase_restart(struct brcms_phy *pi);

-bool wlc_phy_attach_nphy(struct brcms_phy *pi);
+void wlc_phy_attach_nphy(struct brcms_phy *pi);
bool wlc_phy_attach_lcnphy(struct brcms_phy *pi);

void wlc_phy_detach_lcnphy(struct brcms_phy *pi);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
index 8580a2754789..cd9b502a6a9f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
@@ -14546,7 +14546,7 @@ static void wlc_phy_txpwr_srom_read_ppr_nphy(struct brcms_phy *pi)
wlc_phy_txpwr_apply_nphy(pi);
}

-static bool wlc_phy_txpwr_srom_read_nphy(struct brcms_phy *pi)
+static void wlc_phy_txpwr_srom_read_nphy(struct brcms_phy *pi)
{
struct ssb_sprom *sprom = &pi->d11core->bus->sprom;

@@ -14595,11 +14595,9 @@ static bool wlc_phy_txpwr_srom_read_nphy(struct brcms_phy *pi)
pi->phycal_tempdelta = 0;

wlc_phy_txpwr_srom_read_ppr_nphy(pi);
-
- return true;
}

-bool wlc_phy_attach_nphy(struct brcms_phy *pi)
+void wlc_phy_attach_nphy(struct brcms_phy *pi)
{
uint i;

@@ -14645,10 +14643,7 @@ bool wlc_phy_attach_nphy(struct brcms_phy *pi)
pi->pi_fptr.chanset = wlc_phy_chanspec_set_nphy;
pi->pi_fptr.txpwrrecalc = wlc_phy_txpower_recalc_target_nphy;

- if (!wlc_phy_txpwr_srom_read_nphy(pi))
- return false;
-
- return true;
+ wlc_phy_txpwr_srom_read_nphy(pi);
}

static s32 get_rf_pwr_offset(struct brcms_phy *pi, s16 pga_gn, s16 pad_gn)
--
2.37.3