| Message ID | 20210318164513.19600-1-colin.king@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | brcmsmac: fix shift on 4 bit masked value | expand |
Colin King <colin.king@canonical.com> writes: > From: Colin Ian King <colin.king@canonical.com> > > The calculation of offtune_val seems incorrect, the u16 value in > pi->tx_rx_cal_radio_saveregs[2] is being masked with 0xf0 and then > shifted 8 places right so that always ends up as a zero result. I > believe the intended shift was 4 bits to the right. Fix this. > > [Note: not tested, I don't have the H/W] > > Addresses-Coverity: ("Operands don't affect result") > Fixes: 5b435de0d786 ("net: wireless: add brcm80211 drivers") > Signed-off-by: Colin Ian King <colin.king@canonical.com> Can someone ack this? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > The calculation of offtune_val seems incorrect, the u16 value in > pi->tx_rx_cal_radio_saveregs[2] is being masked with 0xf0 and then > shifted 8 places right so that always ends up as a zero result. I > believe the intended shift was 4 bits to the right. Fix this. > > [Note: not tested, I don't have the H/W] > > Addresses-Coverity: ("Operands don't affect result") > Fixes: 5b435de0d786 ("net: wireless: add brcm80211 drivers") > Signed-off-by: Colin Ian King <colin.king@canonical.com> I think this needs review from someone familiar with the hardware. Patch set to Changes Requested.
On Sun, 2021-04-18 at 06:10 +0000, Kalle Valo wrote: > Colin King <colin.king@canonical.com> wrote: > > > From: Colin Ian King <colin.king@canonical.com> > > > > The calculation of offtune_val seems incorrect, the u16 value in > > pi->tx_rx_cal_radio_saveregs[2] is being masked with 0xf0 and then > > shifted 8 places right so that always ends up as a zero result. I > > believe the intended shift was 4 bits to the right. Fix this. > > > > [Note: not tested, I don't have the H/W] > > > > Addresses-Coverity: ("Operands don't affect result") > > Fixes: 5b435de0d786 ("net: wireless: add brcm80211 drivers") > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > I think this needs review from someone familiar with the hardware. > > Patch set to Changes Requested. What "change" are you requesting here? Likely there needs to be some other setting for the patch. Perhaps "deferred" as you seem to be requesting a review and there's no actual change necessary, just approval from someone with the hardware and that someone test the patch.
Joe Perches <joe@perches.com> writes: > On Sun, 2021-04-18 at 06:10 +0000, Kalle Valo wrote: >> Colin King <colin.king@canonical.com> wrote: >> >> > From: Colin Ian King <colin.king@canonical.com> >> > >> > The calculation of offtune_val seems incorrect, the u16 value in >> > pi->tx_rx_cal_radio_saveregs[2] is being masked with 0xf0 and then >> > shifted 8 places right so that always ends up as a zero result. I >> > believe the intended shift was 4 bits to the right. Fix this. >> > >> > [Note: not tested, I don't have the H/W] >> > >> > Addresses-Coverity: ("Operands don't affect result") >> > Fixes: 5b435de0d786 ("net: wireless: add brcm80211 drivers") >> > Signed-off-by: Colin Ian King <colin.king@canonical.com> >> >> I think this needs review from someone familiar with the hardware. >> >> Patch set to Changes Requested. > > What "change" are you requesting here? Don't take patchwork states literally, a better name for this state would be "Needs work" or something like that. > Likely there needs to be some other setting for the patch. > > Perhaps "deferred" as you seem to be requesting a review > and there's no actual change necessary, just approval from > someone with the hardware and that someone test the patch. I already asked for help on April 7th and nobody replied, so I'm dropping this now. The patch can be resent if/when the change is confirmed to be correct. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
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..2c04bae6e21c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c @@ -26329,7 +26329,7 @@ static void wlc_phy_rxcal_radio_setup_nphy(struct brcms_phy *pi, u8 rx_core) offtune_val = (pi->tx_rx_cal_radio_saveregs - [2] & 0xF0) >> 8; + [2] & 0xF0) >> 4; offtune_val = (offtune_val <= 0x7) ? 0xF : 0;