diff mbox series

wifi: rtw88: Report the signal strength only if it's known

Message ID f7e1e448-2c9b-498f-b8b1-a14dd967d7d3@gmail.com (mailing list archive)
State Accepted
Delegated to: Ping-Ke Shih
Headers show
Series wifi: rtw88: Report the signal strength only if it's known | expand

Commit Message

Bitterblue Smith Oct. 22, 2024, 5:20 p.m. UTC
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(-)

Comments

Ping-Ke Shih Oct. 23, 2024, 1:57 a.m. UTC | #1
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
Bitterblue Smith Oct. 23, 2024, 1:16 p.m. UTC | #2
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
>
Ping-Ke Shih Oct. 24, 2024, 1:17 a.m. UTC | #3
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.
Ping-Ke Shih Oct. 29, 2024, 3:38 a.m. UTC | #4
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>

1 patch(es) applied to rtw-next branch of rtw.git, thanks.

47f754b3f838 wifi: rtw88: Report the signal strength only if it's known

---
https://github.com/pkshih/rtw.git
diff mbox series

Patch

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);