| Message ID | f7e1e448-2c9b-498f-b8b1-a14dd967d7d3@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Ping-Ke Shih |
| Headers | show |
| Series | wifi: rtw88: Report the signal strength only if it's known | expand |
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > > RTL8811CU doesn't report the signal strength for many (any?) data > frames. When the signal strength is not known, set > RX_FLAG_NO_SIGNAL_VAL in order to avoid reporting a signal > strength of 0. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> Acked-by: Ping-Ke Shih <pkshih@realtek.com> > --- > drivers/net/wireless/realtek/rtw88/rx.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c > index 1de93fc9efe9..90fc8a5fa89e 100644 > --- a/drivers/net/wireless/realtek/rtw88/rx.c > +++ b/drivers/net/wireless/realtek/rtw88/rx.c > @@ -234,10 +234,14 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev, > else > rx_status->bw = RATE_INFO_BW_20; > > - rx_status->signal = pkt_stat->signal_power; > - for (path = 0; path < rtwdev->hal.rf_path_num; path++) { > - rx_status->chains |= BIT(path); > - rx_status->chain_signal[path] = pkt_stat->rx_power[path]; > + if (pkt_stat->phy_status) { When only pkt_stat->phy_status is set, driver calls rtwdev->chip->ops->query_phy_status() to get PHY status. Have you reviewed all places accessing PHY fields also check pkt_stat->phy_status? > + rx_status->signal = pkt_stat->signal_power; > + for (path = 0; path < rtwdev->hal.rf_path_num; path++) { > + rx_status->chains |= BIT(path); > + rx_status->chain_signal[path] = pkt_stat->rx_power[path]; > + } > + } else { > + rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL; > } > > rtw_rx_addr_match(rtwdev, pkt_stat, hdr); > -- > 2.46.0
On 23/10/2024 04:57, Ping-Ke Shih wrote: > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: >> >> RTL8811CU doesn't report the signal strength for many (any?) data >> frames. When the signal strength is not known, set >> RX_FLAG_NO_SIGNAL_VAL in order to avoid reporting a signal >> strength of 0. >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > > Acked-by: Ping-Ke Shih <pkshih@realtek.com> > >> --- >> drivers/net/wireless/realtek/rtw88/rx.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c >> index 1de93fc9efe9..90fc8a5fa89e 100644 >> --- a/drivers/net/wireless/realtek/rtw88/rx.c >> +++ b/drivers/net/wireless/realtek/rtw88/rx.c >> @@ -234,10 +234,14 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev, >> else >> rx_status->bw = RATE_INFO_BW_20; >> >> - rx_status->signal = pkt_stat->signal_power; >> - for (path = 0; path < rtwdev->hal.rf_path_num; path++) { >> - rx_status->chains |= BIT(path); >> - rx_status->chain_signal[path] = pkt_stat->rx_power[path]; >> + if (pkt_stat->phy_status) { > > When only pkt_stat->phy_status is set, driver calls rtwdev->chip->ops->query_phy_status() > to get PHY status. Have you reviewed all places accessing PHY fields > also check pkt_stat->phy_status? > Yes, I think this was the only place that didn't check pkt_stat->phy_status. > >> + rx_status->signal = pkt_stat->signal_power; >> + for (path = 0; path < rtwdev->hal.rf_path_num; path++) { >> + rx_status->chains |= BIT(path); >> + rx_status->chain_signal[path] = pkt_stat->rx_power[path]; >> + } >> + } else { >> + rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL; >> } >> >> rtw_rx_addr_match(rtwdev, pkt_stat, hdr); >> -- >> 2.46.0 >
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > On 23/10/2024 04:57, Ping-Ke Shih wrote: > > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > >> diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c > >> index 1de93fc9efe9..90fc8a5fa89e 100644 > >> --- a/drivers/net/wireless/realtek/rtw88/rx.c > >> +++ b/drivers/net/wireless/realtek/rtw88/rx.c > >> @@ -234,10 +234,14 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev, > >> else > >> rx_status->bw = RATE_INFO_BW_20; > >> > >> - rx_status->signal = pkt_stat->signal_power; > >> - for (path = 0; path < rtwdev->hal.rf_path_num; path++) { > >> - rx_status->chains |= BIT(path); > >> - rx_status->chain_signal[path] = pkt_stat->rx_power[path]; > >> + if (pkt_stat->phy_status) { > > > > When only pkt_stat->phy_status is set, driver calls rtwdev->chip->ops->query_phy_status() > > to get PHY status. Have you reviewed all places accessing PHY fields > > also check pkt_stat->phy_status? > > > > Yes, I think this was the only place that didn't check pkt_stat->phy_status. > Great! Thanks for the confirmation.
diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c index 1de93fc9efe9..90fc8a5fa89e 100644 --- a/drivers/net/wireless/realtek/rtw88/rx.c +++ b/drivers/net/wireless/realtek/rtw88/rx.c @@ -234,10 +234,14 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev, else rx_status->bw = RATE_INFO_BW_20; - rx_status->signal = pkt_stat->signal_power; - for (path = 0; path < rtwdev->hal.rf_path_num; path++) { - rx_status->chains |= BIT(path); - rx_status->chain_signal[path] = pkt_stat->rx_power[path]; + if (pkt_stat->phy_status) { + rx_status->signal = pkt_stat->signal_power; + for (path = 0; path < rtwdev->hal.rf_path_num; path++) { + rx_status->chains |= BIT(path); + rx_status->chain_signal[path] = pkt_stat->rx_power[path]; + } + } else { + rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL; } rtw_rx_addr_match(rtwdev, pkt_stat, hdr);
RTL8811CU doesn't report the signal strength for many (any?) data frames. When the signal strength is not known, set RX_FLAG_NO_SIGNAL_VAL in order to avoid reporting a signal strength of 0. Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- drivers/net/wireless/realtek/rtw88/rx.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)