RE: [PATCH wireless 1/2] wifi: rtw88: program the channel when the device is started

[Ping-Ke Shih <pkshih@realtek.com>]

Jeremy Fareau <jeremy.fareau@gmail.com> wrote:
> The chip is reset by power_off()/power_on(), but mac80211 only calls
> ieee80211_ops::config() when its own channel state changes.  After a
> stop/start cycle, such as
> 
>     ip link set <dev> down
>     ip link set <dev> up
> 
> the channel is unchanged from mac80211's point of view, so the radio is
> never reprogrammed.

Not sure why you use 'program' I have never seen the term using to set channel.

>  The device then listens on whatever channel the
> hardware came up on while iw reports the configured one, and receives
> almost nothing.
> 
> Observed with an RTL8814AU (ALFA AWUS1900) in monitor mode on an
> aarch64 host.

Only monitor mode doesn't work as your expectation, right?

>  A 25 s capture on channel 6 after a down/up cycle yields
> 3 frames, and 431 frames as soon as any real channel change is
> requested.  An RTL8812AU (Linksys WUSB6300) on the same host, the same
> channel and the same second yields 2438 frames.
> 
> Program the channel in rtw_ops_start(), like rtw_ips_pwr_up() already
> does when leaving IPS.
> 
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Assisted-by: LLM
> Signed-off-by: Jeremy Fareau <jeremy.fareau@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtw88/mac80211.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c
> b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index b01b98d24b0a..827f38390969 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -57,6 +57,17 @@ static int rtw_ops_start(struct ieee80211_hw *hw)
> 
>         mutex_lock(&rtwdev->mutex);
>         ret = rtw_core_start(rtwdev);
> +
> +       /* The chip is reset by power_off()/power_on(), but mac80211 only calls
> +        * ieee80211_ops::config() when its own channel state changes. After a
> +        * stop/start cycle the channel is unchanged from mac80211's point of
> +        * view, so the radio would never be reprogrammed and the device would
> +        * receive nothing. Program it here, like rtw_ips_pwr_up() already does
> +        * when leaving IPS.
> +        */
> +       if (!ret && hw->conf.chandef.chan)

I think we can flag that it is working on monitor mode in
rtw_ops_{add,remove,change}_interface and do set_channel() only if monitor mode.

I suppose we can have a name containing 'monitor_mode', so the comment is not
necessary. 

An existing flag is "rtwdev->hw->conf.flags & IEEE80211_CONF_MONITOR".
I'm not sure if it is enough here. 

> +               rtw_set_channel(rtwdev);
> +
>         mutex_unlock(&rtwdev->mutex);
> 
>         return ret;
> --
> 2.43.0