On Mon, Mar 9, 2020 at 3:59 PM <yhchuang@xxxxxxxxxxx> wrote:
>
> From: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx>
>
> Add a new entry "coex_info" in debugfs to dump coex's states for
> us to debug on coex's issues.
>
> The basic concept for co-existence (coex, usually for WiFi + BT)
> is to decide a strategy based on the current status of WiFi and
> BT. So, it means the WiFi driver requires to gather information
> from BT side and choose a strategy (TDMA/table/HW settings).
>
> Althrough we can easily check the current status of WiFi, e.g.,
> from kernel log or just dump the hardware registers, it is still
> very difficult for us to gather so many different types of WiFi
> states (such as RFE config, antenna, channel/band, TRX, Power
> save). Also we will need BT's information that is stored in
> "struct rtw_coex". So it is necessary for us to have a debugfs
> that can dump all of the WiFi/BT information required.
>
> Note that to debug on coex related issues, we usually need a
> longer period of time of coex_info dump every 2 seconds (for
> example, 30 secs, so we should have 15 times of coex_info's
> dump).
>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx>
> ---
>
> v1 -> v2
> * don't ignore "ignore wlan command"
>
> drivers/net/wireless/realtek/rtw88/coex.c | 492 ++++++++++++++++++
> drivers/net/wireless/realtek/rtw88/coex.h | 10 +
> drivers/net/wireless/realtek/rtw88/debug.c | 17 +
> drivers/net/wireless/realtek/rtw88/main.h | 18 +
> drivers/net/wireless/realtek/rtw88/rtw8822b.c | 30 ++
> drivers/net/wireless/realtek/rtw88/rtw8822c.c | 28 +
> 6 files changed, 595 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c
> index f91dc21a8bf1..2dd95c8acd93 100644
> --- a/drivers/net/wireless/realtek/rtw88/coex.c
> +++ b/drivers/net/wireless/realtek/rtw88/coex.c
> +#ifdef CONFIG_RTW88_DEBUGFS
> +#define INFO_SIZE 80
> +
> +static int rtw_coex_addr_info(struct rtw_dev *rtwdev,
> + const struct rtw_reg_domain *reg,
> + char addr_info[], int n)
> +{
> + const char *rf_prefix = "";
> + const char *sep = n == 0 ? "" : "/ ";
> + int ffs, fls;
> + int max_fls;
> +
> + if (INFO_SIZE - n <= 0)
> + return 0;
> +
> + switch (reg->domain) {
> + case RTW_REG_DOMAIN_MAC32:
> + max_fls = 31;
> + break;
> + case RTW_REG_DOMAIN_MAC16:
> + max_fls = 15;
> + break;
> + case RTW_REG_DOMAIN_MAC8:
> + max_fls = 7;
> + break;
> + case RTW_REG_DOMAIN_RF_A:
> + case RTW_REG_DOMAIN_RF_B:
> + rf_prefix = "RF_";
> + max_fls = 19;
> + break;
> + default:
> + return 0;
> + }
> +
> + ffs = __ffs(reg->mask);
> + fls = __fls(reg->mask);
> +
> + if (ffs == 0 && fls == max_fls)
> + return snprintf(addr_info + n, INFO_SIZE - n, "%s%s%x",
> + sep, rf_prefix, reg->addr);
> + else if (ffs == fls)
> + return snprintf(addr_info + n, INFO_SIZE - n, "%s%s%x[%d]",
> + sep, rf_prefix, reg->addr, ffs);
> + else
> + return snprintf(addr_info + n, INFO_SIZE - n, "%s%s%x[%d:%d]",
> + sep, rf_prefix, reg->addr, fls, ffs);
> +}
> +
> +static int rtw_coex_val_info(struct rtw_dev *rtwdev,
> + const struct rtw_reg_domain *reg,
> + char val_info[], int n)
> +{
> + const char *sep = n == 0 ? "" : "/ ";
> + u8 rf_path;
> +
> + if (INFO_SIZE - n <= 0)
> + return 0;
> +
> + switch (reg->domain) {
> + case RTW_REG_DOMAIN_MAC32:
> + return snprintf(val_info + n, INFO_SIZE - n, "%s0x%x", sep,
> + rtw_read32_mask(rtwdev, reg->addr, reg->mask));
> + case RTW_REG_DOMAIN_MAC16:
> + return snprintf(val_info + n, INFO_SIZE - n, "%s0x%x", sep,
> + rtw_read16_mask(rtwdev, reg->addr, reg->mask));
> + case RTW_REG_DOMAIN_MAC8:
> + return snprintf(val_info + n, INFO_SIZE - n, "%s0x%x", sep,
> + rtw_read8_mask(rtwdev, reg->addr, reg->mask));
> + case RTW_REG_DOMAIN_RF_A:
> + rf_path = RF_PATH_A;
> + break;
> + case RTW_REG_DOMAIN_RF_B:
> + rf_path = RF_PATH_B;
> + break;
> + default:
> + return 0;
> + }
> +
> + /* only RF go through here */
> + return snprintf(val_info + n, INFO_SIZE - n, "%s0x%x", sep,
> + rtw_read_rf(rtwdev, rf_path, reg->addr, reg->mask));
> +}
> +
> +static void rtw_coex_set_coexinfo_hw(struct rtw_dev *rtwdev, struct seq_file *m)
> +{
> + struct rtw_chip_info *chip = rtwdev->chip;
> + const struct rtw_reg_domain *reg;
> + char addr_info[INFO_SIZE];
> + int n_addr = 0;
> + char val_info[INFO_SIZE];
> + int n_val = 0;
> + int i;
> +
> + for (i = 0; i < chip->coex_info_hw_regs_num; i++) {
> + reg = &chip->coex_info_hw_regs[i];
> +
> + n_addr += rtw_coex_addr_info(rtwdev, reg, addr_info, n_addr);
> + n_val += rtw_coex_val_info(rtwdev, reg, val_info, n_val);
> +
> + if (reg->domain == RTW_REG_DOMAIN_NL) {
> + seq_printf(m, "%-40s = %s\n", addr_info, val_info);
> + n_addr = 0;
> + n_val = 0;
> + }
> + }
> +
> + if (n_addr != 0 && n_val != 0)
> + seq_printf(m, "%-40s = %s\n", addr_info, val_info);
> +}
> +
Looks good to me. But I strongly suggest using scnprintf instead of
snprintf to prevent
potential buffer overflow.
Chris
