Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Authenticated-By: 
From:   Ping-Ke Shih <pkshih@realtek.com>
To:     Martin Kaistra <martin.kaistra@linutronix.de>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
CC:     Jes Sorensen <Jes.Sorensen@gmail.com>,
        Kalle Valo <kvalo@kernel.org>,
        Bitterblue Smith <rtl8821cerfe2@gmail.com>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: RE: [PATCH v3 03/18] wifi: rtl8xxxu: Add beacon functions
Thread-Topic: [PATCH v3 03/18] wifi: rtl8xxxu: Add beacon functions
Thread-Index: AQHZeOgGGoRHEtLeQ0Ksj5W7CE3Kna8/4FpA
Date:   Fri, 28 Apr 2023 00:38:38 +0000
Message-ID: <39b41578a55e4375a6bdd5b127a3f20b@realtek.com>
References: <20230427090922.165088-1-martin.kaistra@linutronix.de>
 <20230427090922.165088-4-martin.kaistra@linutronix.de>
In-Reply-To: <20230427090922.165088-4-martin.kaistra@linutronix.de>
Accept-Language: en-US, zh-TW
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Precedence: bulk


> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Thursday, April 27, 2023 5:09 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH v3 03/18] wifi: rtl8xxxu: Add beacon functions
> 
> Add a workqueue to update the beacon contents asynchronously and
> implement downloading the beacon to the HW and starting beacon tx like
> the vendor driver.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---

[...]

>  /*
>   * The rtl8723a has 3 channel groups for it's efuse settings. It only
> @@ -4964,6 +4978,16 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>                 dev_dbg(dev, "Changed BASIC_RATES!\n");
>                 rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
>         }
> +
> +       if (changed & BSS_CHANGED_BEACON_ENABLED && bss_conf->enable_beacon)
> +               rtl8xxxu_start_tx_beacon(priv);
> +
> +       if (changed & BSS_CHANGED_BEACON)
> +               schedule_work(&priv->update_beacon_work);
> +
> +       if (changed & BSS_CHANGED_BEACON_ENABLED && !bss_conf->enable_beacon)
> +               rtl8xxxu_stop_tx_beacon(priv);
> +

I thought these three synchronous works, so I have this suggestion, but actually
update_beacon_work is asynchronous. Then, we can start/stop tx_beacon into a
branch.

	if (changed & BSS_CHANGED_BEACON_ENABLED) {
		if (bss_conf->enable_beacon)
			rtl8xxxu_start_tx_beacon(priv);
		else
			rtl8xxxu_stop_tx_beacon(priv);
	}

	if (changed & BSS_CHANGED_BEACON)
		schedule_work(&priv->update_beacon_work);

Ping-Ke