[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <467fd994-01f0-4057-964c-4184a23e59c4@broadcom.com>
Date: Wed, 30 Oct 2024 12:08:07 +0100
From: Arend van Spriel <arend.vanspriel@...adcom.com>
To: "Colin King (gmail)" <colin.i.king@...il.com>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
brcm80211@...ts.linux.dev, brcm80211-dev-list.pdl@...adcom.com
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Dmitry Antipov <dmantipov@...dex.ru>
Subject: Re: incorrect shift and mask operation in
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
On 10/15/2024 1:02 AM, Colin King (gmail) wrote:
> Hi,
>
> Static analysis on drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/
> phy_n.c has found an issue with a mask and shift operation in function
> wlc_phy_rxcal_radio_setup_nphy() as follows:
>
> lines 26326-26330:
>
> offtune_val =
> (pi->tx_rx_cal_radio_saveregs
> [2] & 0xF0) >> 8;
> offtune_val =
> (offtune_val <= 0x7) ? 0xF : 0;
>
> and similar in lines 26376-26381 too.
>
> The issue is that the expression pi->tx_rx_cal_radio_saveregs[2] & 0xF0
> when shifted 8 places right is always zero, so this looks like a mistake
> since some value value between 0..0xf is expected in the second statement.
>
> Since pi->tx_rx_cal_radio_saveregs[2] is a u16 value the expression
> could plausible be:
>
> (pi->tx_rx_cal_radio_saveregs[2] & 0xf0) >> 4
> or
> (pi->tx_rx_cal_radio_saveregs[2] & 0xf00) >> 8
>
> I don't have knowledge of the hardware so I'm not sure what a suitable
> fix is.
Thanks, Colin
That looks pretty redundant indeed. I look into the history of it, but
ended up in an old end-of-life development tree and the code was pretty
much the same. It is one of the finer dark arts of our phy/radio
development team I guess ;-)
I see that Dmitry Antipov submitted a patch to address this. Again
thanks for reporting.
Regards,
Arend
Powered by blists - more mailing lists