-----Original Message-----
From: kvalo=codeaurora.org@mg.codeaurora.org [mailto:kvalo=codeaurora.org@mg.codeaurora.org] On
Behalf Of Kalle Valo
Sent: Sunday, August 29, 2021 7:40 PM
To: Pkshih
Cc: tony0620emma@gmail.com; linux-wireless@vger.kernel.org; Timlee; Kevin Yang
Subject: Re: [PATCH 3/4] rtw88: support adaptivity for ETSI/JP DFS region

Ping-Ke Shih [off-list ref] writes:

quoted

From: Zong-Zhe Yang <redacted>

Add Energy Detected CCA (EDCCA) mechanism to detect energy on the channel.
And EDCCA support adaptivity mode now. From MIC Ordinance Regulating Radio
Equipment article 49.20, ETSI EN-300-328 and EN-301-893, the device should
be able to dynamically pause TX activity when energy detected on the air.
According to ETSI/JP DFS region, driver will set corresponding threshold
and stop TX activity if the detected energy exceeds the threshold. For now,
we support it on 8822b and 8822c first.

By default, EDCCA mechanism is turned on. For ETSI/JP DFS region, it will
turn to adaptivity mode. However, with adaptivity, if environment is too
nosiy, TX may often be halted. So, a debugfs for EDCCA is added. It can
show what EDCCA mode is used currently. And EDCCA mechanism can be turned
on/off through the debugfs while debugging.

Signed-off-by: Zong-Zhe Yang <redacted>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

[...]

quoted

+static ssize_t rtw_debugfs_set_edcca_enable(struct file *filp,
+					    const char __user *buffer,
+					    size_t count, loff_t *loff)
+{
+	struct seq_file *seqpriv = (struct seq_file *)filp->private_data;
+	struct rtw_debugfs_priv *debugfs_priv = seqpriv->private;
+	struct rtw_dev *rtwdev = debugfs_priv->rtwdev;
+	bool input;
+	int err;
+
+	err = kstrtobool_from_user(buffer, count, &input);
+	if (err)
+		return err;
+
+	rtw_edcca_enabled = input;
+	rtw_phy_adaptivity_set_mode(rtwdev);
+
+	return count;
+}
+
+static int rtw_debugfs_get_edcca_enable(struct seq_file *m, void *v)
+{
+	struct rtw_debugfs_priv *debugfs_priv = m->private;
+	struct rtw_dev *rtwdev = debugfs_priv->rtwdev;
+	struct rtw_dm_info *dm_info = &rtwdev->dm_info;
+
+	seq_printf(m, "EDCCA %s: EDCCA mode %d\n",
+		   rtw_edcca_enabled ? "enabled" : "disabled",
+		   dm_info->edcca_mode);
+	return 0;
+}

[...]

quoted

--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c

@@ -23,6 +23,7 @@ EXPORT_SYMBOL(rtw_disable_lps_deep_mode);
 bool rtw_bf_support = true;
 unsigned int rtw_debug_mask;
 EXPORT_SYMBOL(rtw_debug_mask);
+bool rtw_edcca_enabled = true;

You are making rtw_edcca_enabled per driver, should it instead be per
device? If something is changed via debugfs the assumption is that the
value is per device.

For example, let's say you have two rtw88 devices attached on the same
system, device A and B. If a user changes edcca via debugfs on device A
it will also change the state in device B. Is that the desired
functionality? If yes, I think you should add a comment to
rtw_edcca_enabled explaining that.

As mentioned in commit message, the debugfs is expected to be used when debugging
in noisy environment. In that case, we think all rtw88 devices will probably
be affected. Besides, we believe that turning EDCCA off is a temporary state under
debugging.

Based on the two points, it seems no need to maintain rtw_edcca_enabled by device,
so a comment will be added to explain this.

--
Ping-Ke