* Re: [PATCH][next] rtw88: fix shift of more than 32 bits of a integer
From: Kalle Valo @ 2019-05-02 14:58 UTC (permalink / raw)
To: Colin King
Cc: Yan-Hsuan Chuang, David S . Miller, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190501141945.22522-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the shift of an integer value more than 32 bits can
> occur when nss is more than 32. Fix this by making the integer
> constants unsigned long longs before shifting and bit-wise or'ing
> with the u64 ra_mask to avoid the undefined shift behaviour.
>
> Addresses-Coverity: ("Bad shift operation")
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
b85bd9a14c4b rtw88: fix shift of more than 32 bits of a integer
--
https://patchwork.kernel.org/patch/10925147/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH][next] rtw88: phy: mark expected switch fall-throughs
From: Kalle Valo @ 2019-05-02 14:59 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Yan-Hsuan Chuang, David S. Miller, linux-wireless, netdev,
linux-kernel, Gustavo A. R. Silva, Kees Cook
In-Reply-To: <20190501151615.GA18557@embeddedor>
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
>
> This patch fixes the following warnings:
>
> drivers/net/wireless/realtek/rtw88/phy.c: In function ‘rtw_get_channel_group’:
> ./include/linux/compiler.h:77:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
> # define unlikely(x) __builtin_expect(!!(x), 0)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/asm-generic/bug.h:125:2: note: in expansion of macro ‘unlikely’
> unlikely(__ret_warn_on); \
> ^~~~~~~~
> drivers/net/wireless/realtek/rtw88/phy.c:907:3: note: in expansion of macro ‘WARN_ON’
> WARN_ON(1);
> ^~~~~~~
> drivers/net/wireless/realtek/rtw88/phy.c:908:2: note: here
> case 1:
> ^~~~
> In file included from ./include/linux/bcd.h:5,
> from drivers/net/wireless/realtek/rtw88/phy.c:5:
> drivers/net/wireless/realtek/rtw88/phy.c: In function ‘phy_get_2g_tx_power_index’:
> ./include/linux/compiler.h:77:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
> # define unlikely(x) __builtin_expect(!!(x), 0)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/asm-generic/bug.h:125:2: note: in expansion of macro ‘unlikely’
> unlikely(__ret_warn_on); \
> ^~~~~~~~
> drivers/net/wireless/realtek/rtw88/phy.c:1021:3: note: in expansion of macro ‘WARN_ON’
> WARN_ON(1);
> ^~~~~~~
> drivers/net/wireless/realtek/rtw88/phy.c:1022:2: note: here
> case RTW_CHANNEL_WIDTH_20:
> ^~~~
>
> Warning level 3 was used: -Wimplicit-fallthrough=3
>
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Patch applied to wireless-drivers-next.git, thanks.
aa8eaaaa123a rtw88: phy: mark expected switch fall-throughs
--
https://patchwork.kernel.org/patch/10925201/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH] rtw88: Made RA_MASK macros ULL
From: Nathan Chancellor @ 2019-05-02 15:00 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, clang-built-linux,
Nick Desaulniers, Nathan Chancellor
Clang warns about the definitions of these macros (full warnings trimmed
for brevity):
drivers/net/wireless/realtek/rtw88/main.c:524:15: warning: signed shift
result (0x3FF00000000) requires 43 bits to represent, but 'int' only has
32 bits [-Wshift-overflow]
ra_mask &= RA_MASK_VHT_RATES | RA_MASK_OFDM_IN_VHT;
^~~~~~~~~~~~~~~~~
drivers/net/wireless/realtek/rtw88/main.c:527:15: warning: signed shift
result (0xFF0000000) requires 37 bits to represent, but 'int' only has
32 bits [-Wshift-overflow]
ra_mask &= RA_MASK_HT_RATES | RA_MASK_OFDM_IN_HT_5G;
^~~~~~~~~~~~~~~~
Given that these are all used with ra_mask, which is of type u64, we can
just declare the macros to be ULL as well.
Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Link: https://github.com/ClangBuiltLinux/linux/issues/467
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/net/wireless/realtek/rtw88/main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 9893e5e297e3..a14a5f1b4b6d 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -462,15 +462,15 @@ static u8 get_rate_id(u8 wireless_set, enum rtw_bandwidth bw_mode, u8 tx_num)
#define RA_MASK_CCK_RATES 0x0000f
#define RA_MASK_OFDM_RATES 0x00ff0
-#define RA_MASK_HT_RATES_1SS (0xff000 << 0)
-#define RA_MASK_HT_RATES_2SS (0xff000 << 8)
-#define RA_MASK_HT_RATES_3SS (0xff000 << 16)
+#define RA_MASK_HT_RATES_1SS (0xff000ULL << 0)
+#define RA_MASK_HT_RATES_2SS (0xff000ULL << 8)
+#define RA_MASK_HT_RATES_3SS (0xff000ULL << 16)
#define RA_MASK_HT_RATES (RA_MASK_HT_RATES_1SS | \
RA_MASK_HT_RATES_2SS | \
RA_MASK_HT_RATES_3SS)
-#define RA_MASK_VHT_RATES_1SS (0x3ff000 << 0)
-#define RA_MASK_VHT_RATES_2SS (0x3ff000 << 10)
-#define RA_MASK_VHT_RATES_3SS (0x3ff000 << 20)
+#define RA_MASK_VHT_RATES_1SS (0x3ff000ULL << 0)
+#define RA_MASK_VHT_RATES_2SS (0x3ff000ULL << 10)
+#define RA_MASK_VHT_RATES_3SS (0x3ff000ULL << 20)
#define RA_MASK_VHT_RATES (RA_MASK_VHT_RATES_1SS | \
RA_MASK_VHT_RATES_2SS | \
RA_MASK_VHT_RATES_3SS)
--
2.21.0
^ permalink raw reply related
* [PATCH v2] rtw88: Make RA_MASK macros ULL
From: Nathan Chancellor @ 2019-05-02 15:02 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, clang-built-linux,
Nick Desaulniers, Nathan Chancellor
In-Reply-To: <20190502150022.4182-1-natechancellor@gmail.com>
Clang warns about the definitions of these macros (full warnings trimmed
for brevity):
drivers/net/wireless/realtek/rtw88/main.c:524:15: warning: signed shift
result (0x3FF00000000) requires 43 bits to represent, but 'int' only has
32 bits [-Wshift-overflow]
ra_mask &= RA_MASK_VHT_RATES | RA_MASK_OFDM_IN_VHT;
^~~~~~~~~~~~~~~~~
drivers/net/wireless/realtek/rtw88/main.c:527:15: warning: signed shift
result (0xFF0000000) requires 37 bits to represent, but 'int' only has
32 bits [-Wshift-overflow]
ra_mask &= RA_MASK_HT_RATES | RA_MASK_OFDM_IN_HT_5G;
^~~~~~~~~~~~~~~~
Given that these are all used with ra_mask, which is of type u64, we can
just declare the macros to be ULL as well.
Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Link: https://github.com/ClangBuiltLinux/linux/issues/467
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Fix commit message wording (made -> make)...
drivers/net/wireless/realtek/rtw88/main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 9893e5e297e3..a14a5f1b4b6d 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -462,15 +462,15 @@ static u8 get_rate_id(u8 wireless_set, enum rtw_bandwidth bw_mode, u8 tx_num)
#define RA_MASK_CCK_RATES 0x0000f
#define RA_MASK_OFDM_RATES 0x00ff0
-#define RA_MASK_HT_RATES_1SS (0xff000 << 0)
-#define RA_MASK_HT_RATES_2SS (0xff000 << 8)
-#define RA_MASK_HT_RATES_3SS (0xff000 << 16)
+#define RA_MASK_HT_RATES_1SS (0xff000ULL << 0)
+#define RA_MASK_HT_RATES_2SS (0xff000ULL << 8)
+#define RA_MASK_HT_RATES_3SS (0xff000ULL << 16)
#define RA_MASK_HT_RATES (RA_MASK_HT_RATES_1SS | \
RA_MASK_HT_RATES_2SS | \
RA_MASK_HT_RATES_3SS)
-#define RA_MASK_VHT_RATES_1SS (0x3ff000 << 0)
-#define RA_MASK_VHT_RATES_2SS (0x3ff000 << 10)
-#define RA_MASK_VHT_RATES_3SS (0x3ff000 << 20)
+#define RA_MASK_VHT_RATES_1SS (0x3ff000ULL << 0)
+#define RA_MASK_VHT_RATES_2SS (0x3ff000ULL << 10)
+#define RA_MASK_VHT_RATES_3SS (0x3ff000ULL << 20)
#define RA_MASK_VHT_RATES (RA_MASK_VHT_RATES_1SS | \
RA_MASK_VHT_RATES_2SS | \
RA_MASK_VHT_RATES_3SS)
--
2.21.0
^ permalink raw reply related
* [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Nathan Chancellor @ 2019-05-02 15:15 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, clang-built-linux,
Nick Desaulniers, Nathan Chancellor
When building with -Wuninitialized, Clang warns:
drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
is uninitialized when used here [-Wuninitialized]
put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
^~~~
drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
variable 'data' to silence this warning
u8 *data;
^
= NULL
1 warning generated.
Using Clang's suggestion of initializing data to NULL wouldn't work out
because data will be dereferenced by put_unaligned_le32. Use kzalloc to
properly initialize data, which matches a couple of other places in this
driver.
Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
Link: https://github.com/ClangBuiltLinux/linux/issues/464
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index f9c67ed473d1..b35728564c7b 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
u32 addr;
u8 *data;
+ data = kzalloc(sizeof(u32), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
if (status < 0) {
rsi_dbg(ERR_ZONE,
"Unable to set ms word to common reg\n");
- return status;
+ goto err;
}
rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
@@ -944,7 +948,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to hold TA threads\n");
- return status;
+ goto err;
}
put_unaligned_le32(TA_SOFT_RST_CLR, data);
@@ -954,7 +958,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to get TA out of reset\n");
- return status;
+ goto err;
}
put_unaligned_le32(TA_PC_ZERO, data);
@@ -964,7 +968,8 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to Reset TA PC value\n");
- return -EINVAL;
+ status = -EINVAL;
+ goto err;
}
put_unaligned_le32(TA_RELEASE_THREAD_VALUE, data);
@@ -974,17 +979,19 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to release TA threads\n");
- return status;
+ goto err;
}
status = rsi_sdio_master_access_msword(adapter, MISC_CFG_BASE_ADDR);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to set ms word to common reg\n");
- return status;
+ goto err;
}
rsi_dbg(INIT_ZONE, "***** TA Reset done *****\n");
- return 0;
+err:
+ kfree(data);
+ return status;
}
static struct rsi_host_intf_ops sdio_host_intf_ops = {
--
2.21.0
^ permalink raw reply related
* Re: [RFC 10/17] mt7615: mcu: remove skb_ret from mt7615_mcu_msg_send
From: Lorenzo Bianconi @ 2019-05-02 15:31 UTC (permalink / raw)
To: Roy Luo
Cc: nbd, lorenzo.bianconi, linux-wireless,
Ryder Lee (李庚諺)
In-Reply-To: <CA+zupgzQTzQyihC_UskHD=63Ag3AnQcD4_pcas7VdaehyoCE1w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11019 bytes --]
> Hi Lorenzo,
>
Hi Roy,
> IMHO, the skb_ret parameter from mt7615_mcu_msg_send is not just used by
> patch semaphore control.
> The other cmds also got their own event response that can be interpret
> accordingly by their own caller, which can provide the status of the cmd
> (success/fail) and other information that might be helpful case by case.
Do you mean the fw does not put the status code in a 'event struct'
at the beginning of the skb data buffer? What are the cmds that do not use
that approach?
Regards,
Lorenzo
> For now, we just assume every cmd works well and do no error handling,
> which is risky.
> If we are to take FW response into consideration in the future, maybe we
> should keep this parameter.
>
>
> Regards,
> Cheng-Hao (Roy) Luo
>
>
> On Thu, May 2, 2019 at 12:08 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> > Remove skb_ret parameter from mt7615_mcu_msg_send signature since it is
> > actually used just by mt7615_mcu_patch_sem_ctrl. This is a prelimanry
> > patch to use mt76 common mcu API
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > .../net/wireless/mediatek/mt76/mt7615/mcu.c | 77 ++++++-------------
> > 1 file changed, 24 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > index b8d928e8949c..4d1d4c0bc2e2 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > @@ -116,8 +116,7 @@ static int __mt7615_mcu_msg_send(struct mt7615_dev
> > *dev, struct sk_buff *skb,
> > }
> >
> > static int
> > -mt7615_mcu_msg_send(struct mt7615_dev *dev, struct sk_buff *skb,
> > - int cmd, struct sk_buff **skb_ret)
> > +mt7615_mcu_msg_send(struct mt7615_dev *dev, struct sk_buff *skb, int cmd)
> > {
> > unsigned long expires = jiffies + 10 * HZ;
> > struct mt7615_mcu_rxd *rxd;
> > @@ -142,18 +141,11 @@ mt7615_mcu_msg_send(struct mt7615_dev *dev, struct
> > sk_buff *skb,
> > if (seq != rxd->seq)
> > continue;
> >
> > - if (skb_ret) {
> > - int hdr_len = sizeof(*rxd);
> > -
> > - if (!test_bit(MT76_STATE_MCU_RUNNING,
> > - &dev->mt76.state))
> > - hdr_len -= 4;
> > - skb_pull(skb, hdr_len);
> > - *skb_ret = skb;
> > - } else {
> > - dev_kfree_skb(skb);
> > + if (cmd == -MCU_CMD_PATCH_SEM_CONTROL) {
> > + skb_pull(skb, sizeof(*rxd) - 4);
> > + ret = *skb->data;
> > }
> > -
> > + dev_kfree_skb(skb);
> > break;
> > }
> >
> > @@ -177,8 +169,7 @@ static int mt7615_mcu_init_download(struct mt7615_dev
> > *dev, u32 addr,
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb,
> > -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb,
> > -MCU_CMD_TARGET_ADDRESS_LEN_REQ);
> > }
> >
> > static int mt7615_mcu_send_firmware(struct mt7615_dev *dev, const void
> > *data,
> > @@ -219,43 +210,26 @@ static int mt7615_mcu_start_firmware(struct
> > mt7615_dev *dev, u32 addr,
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ, NULL);
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ);
> > }
> >
> > static int mt7615_mcu_restart(struct mt7615_dev *dev)
> > {
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(NULL, 0);
> >
> > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ);
> > }
> >
> > static int mt7615_mcu_patch_sem_ctrl(struct mt7615_dev *dev, bool get)
> > {
> > struct {
> > - __le32 operation;
> > + __le32 op;
> > } req = {
> > - .operation = cpu_to_le32(get ? PATCH_SEM_GET :
> > - PATCH_SEM_RELEASE),
> > + .op = cpu_to_le32(get ? PATCH_SEM_GET : PATCH_SEM_RELEASE),
> > };
> > - struct event {
> > - u8 status;
> > - u8 reserved[3];
> > - } *resp;
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - struct sk_buff *skb_ret;
> > - int ret;
> >
> > - ret = mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_SEM_CONTROL,
> > - &skb_ret);
> > - if (ret)
> > - goto out;
> > -
> > - resp = (struct event *)(skb_ret->data);
> > - ret = resp->status;
> > - dev_kfree_skb(skb_ret);
> > -
> > -out:
> > - return ret;
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_SEM_CONTROL);
> > }
> >
> > static int mt7615_mcu_start_patch(struct mt7615_dev *dev)
> > @@ -268,7 +242,7 @@ static int mt7615_mcu_start_patch(struct mt7615_dev
> > *dev)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_FINISH_REQ,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_FINISH_REQ);
> > }
> >
> > static int mt7615_driver_own(struct mt7615_dev *dev)
> > @@ -554,8 +528,7 @@ int mt7615_mcu_set_eeprom(struct mt7615_dev *dev)
> > for (off = MT_EE_NIC_CONF_0; off < __MT_EE_MAX; off++)
> > data[off - MT_EE_NIC_CONF_0].val = eep[off];
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EFUSE_BUFFER_MODE,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_EFUSE_BUFFER_MODE);
> > }
> >
> > int mt7615_mcu_init_mac(struct mt7615_dev *dev)
> > @@ -570,7 +543,7 @@ int mt7615_mcu_init_mac(struct mt7615_dev *dev)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_MAC_INIT_CTRL,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_MAC_INIT_CTRL);
> > }
> >
> > int mt7615_mcu_set_rts_thresh(struct mt7615_dev *dev, u32 val)
> > @@ -589,7 +562,7 @@ int mt7615_mcu_set_rts_thresh(struct mt7615_dev *dev,
> > u32 val)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PROTECT_CTRL,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PROTECT_CTRL);
> > }
> >
> > int mt7615_mcu_set_wmm(struct mt7615_dev *dev, u8 queue,
> > @@ -627,7 +600,7 @@ int mt7615_mcu_set_wmm(struct mt7615_dev *dev, u8
> > queue,
> > }
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EDCA_UPDATE,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EDCA_UPDATE);
> > }
> >
> > int mt7615_mcu_ctrl_pm_state(struct mt7615_dev *dev, int enter)
> > @@ -657,7 +630,7 @@ int mt7615_mcu_ctrl_pm_state(struct mt7615_dev *dev,
> > int enter)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PM_STATE_CTRL,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PM_STATE_CTRL);
> > }
> >
> > static int __mt7615_mcu_set_dev_info(struct mt7615_dev *dev,
> > @@ -704,8 +677,7 @@ static int __mt7615_mcu_set_dev_info(struct mt7615_dev
> > *dev,
> >
> > memcpy(skb_push(skb, sizeof(req_hdr)), &req_hdr, sizeof(req_hdr));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE);
> > }
> >
> > int mt7615_mcu_set_dev_info(struct mt7615_dev *dev, struct ieee80211_vif
> > *vif,
> > @@ -830,8 +802,7 @@ static int __mt7615_mcu_set_bss_info(struct mt7615_dev
> > *dev,
> > bss_info_tag_handler[i].handler)
> > bss_info_tag_handler[i].handler(dev, bss_info,
> > skb);
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE);
> > }
> >
> > int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> > @@ -914,7 +885,7 @@ __mt7615_mcu_set_wtbl(struct mt7615_dev *dev, int
> > wlan_idx,
> > if (buf && buf_len)
> > memcpy(skb_put(skb, buf_len), buf, buf_len);
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_WTBL_UPDATE,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_WTBL_UPDATE);
> > }
> >
> > static enum mt7615_cipher_type
> > @@ -1092,7 +1063,7 @@ __mt7615_mcu_set_sta_rec(struct mt7615_dev *dev, int
> > bss_idx,
> > if (buf && buf_len)
> > memcpy(skb_put(skb, buf_len), buf, buf_len);
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_STA_REC_UPDATE,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_STA_REC_UPDATE);
> > }
> >
> > int mt7615_mcu_set_sta_rec_bmc(struct mt7615_dev *dev,
> > @@ -1220,7 +1191,7 @@ int mt7615_mcu_set_bcn(struct mt7615_dev *dev,
> > struct ieee80211_vif *vif,
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BCN_OFFLOAD,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BCN_OFFLOAD);
> > }
> >
> > int mt7615_mcu_set_channel(struct mt7615_dev *dev)
> > @@ -1285,12 +1256,12 @@ int mt7615_mcu_set_channel(struct mt7615_dev *dev)
> > memset(req.txpower_sku, 0x3f, 49);
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH,
> > NULL);
> > + ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH);
> > if (ret)
> > return ret;
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_RX_PATH,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_RX_PATH);
> > }
> >
> > int mt7615_mcu_set_ht_cap(struct mt7615_dev *dev, struct ieee80211_vif
> > *vif,
> > --
> > 2.20.1
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2] rtw88: Make RA_MASK macros ULL
From: Kalle Valo @ 2019-05-02 16:26 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Yan-Hsuan Chuang, linux-wireless, netdev, linux-kernel,
clang-built-linux, Nick Desaulniers, Nathan Chancellor
In-Reply-To: <20190502150209.4475-1-natechancellor@gmail.com>
Nathan Chancellor <natechancellor@gmail.com> wrote:
> Clang warns about the definitions of these macros (full warnings trimmed
> for brevity):
>
> drivers/net/wireless/realtek/rtw88/main.c:524:15: warning: signed shift
> result (0x3FF00000000) requires 43 bits to represent, but 'int' only has
> 32 bits [-Wshift-overflow]
> ra_mask &= RA_MASK_VHT_RATES | RA_MASK_OFDM_IN_VHT;
> ^~~~~~~~~~~~~~~~~
> drivers/net/wireless/realtek/rtw88/main.c:527:15: warning: signed shift
> result (0xFF0000000) requires 37 bits to represent, but 'int' only has
> 32 bits [-Wshift-overflow]
> ra_mask &= RA_MASK_HT_RATES | RA_MASK_OFDM_IN_HT_5G;
> ^~~~~~~~~~~~~~~~
>
> Given that these are all used with ra_mask, which is of type u64, we can
> just declare the macros to be ULL as well.
>
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Link: https://github.com/ClangBuiltLinux/linux/issues/467
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
237b47efcdbc rtw88: Make RA_MASK macros ULL
--
https://patchwork.kernel.org/patch/10927105/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Nick Desaulniers @ 2019-05-02 18:18 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo, linux-wireless,
netdev, LKML, clang-built-linux
In-Reply-To: <20190502151548.11143-1-natechancellor@gmail.com>
On Thu, May 2, 2019 at 8:16 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wuninitialized, Clang warns:
>
> drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
> is uninitialized when used here [-Wuninitialized]
> put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
> ^~~~
> drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
> variable 'data' to silence this warning
> u8 *data;
> ^
> = NULL
> 1 warning generated.
>
> Using Clang's suggestion of initializing data to NULL wouldn't work out
> because data will be dereferenced by put_unaligned_le32. Use kzalloc to
> properly initialize data, which matches a couple of other places in this
> driver.
>
> Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
> Link: https://github.com/ClangBuiltLinux/linux/issues/464
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> index f9c67ed473d1..b35728564c7b 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> @@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
> u32 addr;
> u8 *data;
>
> + data = kzalloc(sizeof(u32), GFP_KERNEL);
Something fishy is going on here. We allocate 4 B but declare data as
a u8* (pointer to individual bytes)? In general, dynamically
allocating that few bytes is a code smell; either you meant to just
use the stack, or this memory's lifetime extends past the lifetime of
this stackframe, at which point you probably just meant to stack
allocate space in a higher parent frame and pass this preallocated
memory down to the child frame to get filled in.
Reading through this code, I don't think that the memory is meant to
outlive the stack frame. Is there a reason why we can't just declare
data as:
u8 data [4];
then use ARRAY_SIZE(data) or RSI_9116_REG_SIZE in rsi_reset_chip(),
getting rid of the kzalloc/kfree?
(Sorry, I hate when a simple fixup becomes a "hey let's rewrite all
this code" thus becoming "that guy.")
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH v9 04/14] rtw88: trx files
From: Brian Norris @ 2019-05-02 18:23 UTC (permalink / raw)
To: Kalle Valo
Cc: Larry Finger, Tony Chuang, linux-wireless, Johannes Berg, Pkshih,
Andy Huang, Stanislaw Gruszka, Greg Kroah-Hartman
In-Reply-To: <87a7g6ni31.fsf@kamboji.qca.qualcomm.com>
On Wed, May 1, 2019 at 11:30 AM Kalle Valo <kvalo@codeaurora.org> wrote:
> My comment was about handling firmware commands and events as a byte
> array, not about bitfields. So that instead of accessing 'index + 1' and
> 'index + 4' you should create a proper struct for the command and access
> it using 'cmd->foo' and 'cmd->bar'. Sure, bitfields you still need to
> access using FIELD_GET() or similar but having a struct for commands is
> a lot cleaner approach. And most upstream drivers do this: ath10k,
> ath6kl, iwlwifi, p54 and whatnot.
I think I pushed Tony away from the bitfields (he was using a struct
plus some ugly bitfields / #ifdefs pre-v8), and he ended up with this
(in v8). I noted on the v8 cover letter that one can still use a
struct, but just avoid using bitfields -- so you would still have 'u8'
and '__le32' fields (or similar), and do the right le32_to_cpu()
accessors (sparse will help you) plus FIELD_GET() (for any necessary
bitfields).
Brian
^ permalink raw reply
* Re: [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Nathan Chancellor @ 2019-05-03 3:17 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo, linux-wireless,
netdev, LKML, clang-built-linux
In-Reply-To: <CAKwvOd=nvKGGW5jvN+WFUXzOm9xeiNNUD0F9--9YcpuRmnWWhA@mail.gmail.com>
On Thu, May 02, 2019 at 11:18:01AM -0700, Nick Desaulniers wrote:
> On Thu, May 2, 2019 at 8:16 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wuninitialized, Clang warns:
> >
> > drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
> > is uninitialized when used here [-Wuninitialized]
> > put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
> > ^~~~
> > drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
> > variable 'data' to silence this warning
> > u8 *data;
> > ^
> > = NULL
> > 1 warning generated.
> >
> > Using Clang's suggestion of initializing data to NULL wouldn't work out
> > because data will be dereferenced by put_unaligned_le32. Use kzalloc to
> > properly initialize data, which matches a couple of other places in this
> > driver.
> >
> > Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/464
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> > index f9c67ed473d1..b35728564c7b 100644
> > --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
> > +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> > @@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
> > u32 addr;
> > u8 *data;
> >
> > + data = kzalloc(sizeof(u32), GFP_KERNEL);
>
> Something fishy is going on here. We allocate 4 B but declare data as
> a u8* (pointer to individual bytes)? In general, dynamically
> allocating that few bytes is a code smell; either you meant to just
> use the stack, or this memory's lifetime extends past the lifetime of
> this stackframe, at which point you probably just meant to stack
> allocate space in a higher parent frame and pass this preallocated
> memory down to the child frame to get filled in.
>
> Reading through this code, I don't think that the memory is meant to
> outlive the stack frame. Is there a reason why we can't just declare
> data as:
>
> u8 data [4];
data was __le32 in rsi_reset_chip() before commit f700546682a6 ("rsi:
fix nommu_map_sg overflow kernel panic").
I wonder if this would be okay for this function:
-------------------------------------------------
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index f9c67ed473d1..0330c50ab99c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -927,7 +927,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
{
int status;
u32 addr;
- u8 *data;
+ u8 data;
status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
if (status < 0) {
@@ -937,7 +937,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
}
rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
- put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
+ put_unaligned_le32(TA_HOLD_THREAD_VALUE, &data);
addr = TA_HOLD_THREAD_REG | RSI_SD_REQUEST_MASTER;
status = rsi_sdio_write_register_multiple(adapter, addr,
(u8 *)&data,
@@ -947,7 +947,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
return status;
}
- put_unaligned_le32(TA_SOFT_RST_CLR, data);
+ put_unaligned_le32(TA_SOFT_RST_CLR, &data);
addr = TA_SOFT_RESET_REG | RSI_SD_REQUEST_MASTER;
status = rsi_sdio_write_register_multiple(adapter, addr,
(u8 *)&data,
@@ -957,7 +957,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
return status;
}
- put_unaligned_le32(TA_PC_ZERO, data);
+ put_unaligned_le32(TA_PC_ZERO, &data);
addr = TA_TH0_PC_REG | RSI_SD_REQUEST_MASTER;
status = rsi_sdio_write_register_multiple(adapter, addr,
(u8 *)&data,
@@ -967,7 +967,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
return -EINVAL;
}
- put_unaligned_le32(TA_RELEASE_THREAD_VALUE, data);
+ put_unaligned_le32(TA_RELEASE_THREAD_VALUE, &data);
addr = TA_RELEASE_THREAD_REG | RSI_SD_REQUEST_MASTER;
status = rsi_sdio_write_register_multiple(adapter, addr,
(u8 *)&data,
>
> then use ARRAY_SIZE(data) or RSI_9116_REG_SIZE in rsi_reset_chip(),
> getting rid of the kzalloc/kfree?
>
> (Sorry, I hate when a simple fixup becomes a "hey let's rewrite all
> this code" thus becoming "that guy.")
If we aren't actually improving the code, then why bother? :)
Thank you for the review!
Nathan
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply related
* Re: [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Kalle Valo @ 2019-05-03 4:38 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Amitkumar Karwar, Siva Rebbagondla,
linux-wireless, netdev, LKML, clang-built-linux
In-Reply-To: <CAKwvOd=nvKGGW5jvN+WFUXzOm9xeiNNUD0F9--9YcpuRmnWWhA@mail.gmail.com>
Nick Desaulniers <ndesaulniers@google.com> writes:
> On Thu, May 2, 2019 at 8:16 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
>>
>> When building with -Wuninitialized, Clang warns:
>>
>> drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
>> is uninitialized when used here [-Wuninitialized]
>> put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
>> ^~~~
>> drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
>> variable 'data' to silence this warning
>> u8 *data;
>> ^
>> = NULL
>> 1 warning generated.
>>
>> Using Clang's suggestion of initializing data to NULL wouldn't work out
>> because data will be dereferenced by put_unaligned_le32. Use kzalloc to
>> properly initialize data, which matches a couple of other places in this
>> driver.
>>
>> Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
>> Link: https://github.com/ClangBuiltLinux/linux/issues/464
>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> ---
>> drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
>> 1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> index f9c67ed473d1..b35728564c7b 100644
>> --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> @@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> u32 addr;
>> u8 *data;
>>
>> + data = kzalloc(sizeof(u32), GFP_KERNEL);
>
> Something fishy is going on here. We allocate 4 B but declare data as
> a u8* (pointer to individual bytes)? In general, dynamically
> allocating that few bytes is a code smell; either you meant to just
> use the stack, or this memory's lifetime extends past the lifetime of
> this stackframe, at which point you probably just meant to stack
> allocate space in a higher parent frame and pass this preallocated
> memory down to the child frame to get filled in.
>
> Reading through this code, I don't think that the memory is meant to
> outlive the stack frame. Is there a reason why we can't just declare
> data as:
>
> u8 data [4];
>
> then use ARRAY_SIZE(data) or RSI_9116_REG_SIZE in rsi_reset_chip(),
> getting rid of the kzalloc/kfree?
I haven't checked the details but AFAIK stack variables are not supposed
to be used with DMA. So in that case I think it's ok alloc four bytes,
unless the DMA rules have changed of course. But I didn't check if rsi
is using DMA here, just a general comment.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/9] wil6210: fix spurious interrupts in 3-msi
From: Kalle Valo @ 2019-05-03 5:05 UTC (permalink / raw)
To: Maya Erez; +Cc: Maya Erez, linux-wireless, wil6210
In-Reply-To: <1556293417-27097-2-git-send-email-merez@codeaurora.org>
Maya Erez <merez@codeaurora.org> wrote:
> Interrupt is set in ICM (ICR & ~IMV) rising trigger.
> As the driver masks the IRQ after clearing it, there can
> be a race where an additional spurious interrupt is triggered
> when the driver unmask the IRQ.
> This can happen in case HW triggers an interrupt after the clear
> and before the mask.
>
> To prevent the second spurious interrupt the driver needs to mask the
> IRQ before reading and clearing it.
>
> Signed-off-by: Maya Erez <merez@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
9 patches applied to ath-next branch of ath.git, thanks.
e10b0eddd523 wil6210: fix spurious interrupts in 3-msi
9c6465979276 wil6210: fix _desc access in __wil_tx_vring_tso
0131d1851338 wil6210: add printout of platform capabilities
a7feb56f204f wil6210: add support for multiple sections in brd file
9a874d045473 wil6210: enhancements for descriptor and status ring debugfs
ddf7afdde824 wil6210: fix overwriting max_assoc_sta module param
7441be71ba7e wil6210: fix missed MISC mbox interrupt
d6a553c0c61b wil6210: check rx_buff_mgmt before accessing it
3d0aa9198446 wil6210: remove HALP for Talyn devices
--
https://patchwork.kernel.org/patch/10919277/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/2] ath10k: Fix rate table updation in tx stats
From: Kalle Valo @ 2019-05-03 6:00 UTC (permalink / raw)
To: Rakesh Pillai; +Cc: ath10k, linux-wireless, Rakesh Pillai
In-Reply-To: <1552319602-17795-2-git-send-email-pillair@codeaurora.org>
Rakesh Pillai <pillair@codeaurora.org> wrote:
> The index for updating rate table, which is displayed
> in the tx stats via debugfs, is calculated using the
> bandwidth value. The bandwidth values do not map
> correctly with the bandwidth values shown in the rate table.
>
> Correct the bandwidth value calculation which is used
> to calculate the index for rate table updation for tx stats.
>
> Tested HW: WCN3990
> Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1
>
> Fixes: e88975ca37d1 ("ath10k: dump tx stats in rate table format")
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
Fails to compile:
drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_accumulate_per_peer_tx_stats':
drivers/net/wireless/ath/ath10k/htt_rx.c:3252:14: error: implicit declaration of function 'ath10k_get_bw'; did you mean 'ath10k_get_tid'? [-Werror=implicit-function-declaration]
rtable_bw = ath10k_get_bw(&ar->hw_params, pstats->flags);
^~~~~~~~~~~~~
ath10k_get_tid
cc1: some warnings being treated as errors
make[5]: *** [drivers/net/wireless/ath/ath10k/htt_rx.o] Error 1
make[4]: *** [drivers/net/wireless/ath/ath10k] Error 2
make[3]: *** [drivers/net/wireless/ath] Error 2
make[2]: *** [drivers/net/wireless] Error 2
make[1]: *** [drivers/net] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2
2 patches set to Changes Requested.
10847733 [1/2] ath10k: Fix rate table updation in tx stats
10847737 [2/2] ath10k: Fix NSS tx stats for legacy rates
--
https://patchwork.kernel.org/patch/10847733/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Chris Chiu @ 2019-05-03 6:38 UTC (permalink / raw)
To: Jes Sorensen, kvalo, davem
Cc: linux-wireless, netdev, Linux Kernel, Linux Upstreaming Team
We have 3 laptops which connect the wifi by the same RTL8723BU.
The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
They have the same problem with the in-kernel rtl8xxxu driver, the
iperf (as a client to an ethernet-connected server) gets ~1Mbps.
Nevertheless, the signal strength is reported as around -40dBm,
which is quite good. From the wireshark capture, the tx rate for each
data and qos data packet is only 1Mbps. Compare to the driver from
https://github.com/lwfinger/rtl8723bu, the same iperf test gets ~12
Mbps or more. The signal strength is reported similarly around
-40dBm. That's why we want to find out the cause and improve.
After reading the source code of the rtl8xxxu driver and Larry's, the
major difference is that Larry's driver has a watchdog which will keep
monitoring the signal quality and updating the rate mask just like the
rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
And this kind of watchdog also exists in rtlwifi driver of some specific
chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
the same member function named dm_watchdog and will invoke the
corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
mask.
Thus I created 2 commits and try to do the same thing on rtl8xxxu.
https://github.com/endlessm/linux/commit/503d0b6eb61f25984042b1f00e6293776ae722c7
https://github.com/endlessm/linux/commit/5b06665766d6c3e25cbf649022989a8f3abc83d6
The 1st commit brings a data structure for rate adaptive which will be
useful for determining higher or lower the tx rate. The second commit
adds a watchdog to monitor and update the tx rate mask and tell the
firmware. After applying these commits, the tx rate of each data and
qos data packet will be 39Mbps (MCS4) with the 0xf00000 as its tx
rate mask. The 20th bit ~ 23th bit means MCS4 to MCS7. It means
that the firmware still picks the lowest rate from the rate mask and
explains why the tx rate of data and qos data is always lowest 1Mbps
because the default rate mask passed is almost 0xFFFFFFF ranges
from the basic CCK rate, OFDM rate, and MCS rate. However, with
Larry's driver, the tx rate observed from wireshark under the same
condition is almost 65Mbps or 72Mbps.
I believe the firmware of RTL8723BU may need fix. And I think we
can still bring in the dm_watchdog as rtlwifi to improve from the
driver side. Please leave precious comments for my commits and
suggest what I can do better to get them upstream. Or suggest if
there's any better idea to fix this. Thanks.
Chris
^ permalink raw reply
* Re: [PATCH 1/2] ath10k: Fix rate table updation in tx stats
From: Rakesh Pillai @ 2019-05-03 6:56 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k, linux-wireless
In-Reply-To: <20190503060032.23F1C61195@smtp.codeaurora.org>
Hi Kalle,
This set of patches is dependent on
https://patchwork.kernel.org/patch/10831319/
I can send out v2 for this patchset, marking the dependency, if needed.
Thanks,
Rakesh Pillai.
On 2019-05-03 11:30, Kalle Valo wrote:
> Rakesh Pillai <pillair@codeaurora.org> wrote:
>
>> The index for updating rate table, which is displayed
>> in the tx stats via debugfs, is calculated using the
>> bandwidth value. The bandwidth values do not map
>> correctly with the bandwidth values shown in the rate table.
>>
>> Correct the bandwidth value calculation which is used
>> to calculate the index for rate table updation for tx stats.
>>
>> Tested HW: WCN3990
>> Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1
>>
>> Fixes: e88975ca37d1 ("ath10k: dump tx stats in rate table format")
>> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
>
> Fails to compile:
>
> drivers/net/wireless/ath/ath10k/htt_rx.c: In function
> 'ath10k_accumulate_per_peer_tx_stats':
> drivers/net/wireless/ath/ath10k/htt_rx.c:3252:14: error: implicit
> declaration of function 'ath10k_get_bw'; did you mean
> 'ath10k_get_tid'? [-Werror=implicit-function-declaration]
> rtable_bw = ath10k_get_bw(&ar->hw_params, pstats->flags);
> ^~~~~~~~~~~~~
> ath10k_get_tid
> cc1: some warnings being treated as errors
> make[5]: *** [drivers/net/wireless/ath/ath10k/htt_rx.o] Error 1
> make[4]: *** [drivers/net/wireless/ath/ath10k] Error 2
> make[3]: *** [drivers/net/wireless/ath] Error 2
> make[2]: *** [drivers/net/wireless] Error 2
> make[1]: *** [drivers/net] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [drivers] Error 2
>
> 2 patches set to Changes Requested.
>
> 10847733 [1/2] ath10k: Fix rate table updation in tx stats
> 10847737 [2/2] ath10k: Fix NSS tx stats for legacy rates
^ permalink raw reply
* Re: [PATCH 1/2] ath10k: Fix rate table updation in tx stats
From: Kalle Valo @ 2019-05-03 7:04 UTC (permalink / raw)
To: Rakesh Pillai; +Cc: ath10k, linux-wireless
In-Reply-To: <edfbff30627849996b0597b964ed018c@codeaurora.org>
(please don't top post, fixing that manually)
Rakesh Pillai <pillair@codeaurora.org> writes:
> On 2019-05-03 11:30, Kalle Valo wrote:
>> Rakesh Pillai <pillair@codeaurora.org> wrote:
>>
>>> The index for updating rate table, which is displayed
>>> in the tx stats via debugfs, is calculated using the
>>> bandwidth value. The bandwidth values do not map
>>> correctly with the bandwidth values shown in the rate table.
>>>
>>> Correct the bandwidth value calculation which is used
>>> to calculate the index for rate table updation for tx stats.
>>>
>>> Tested HW: WCN3990
>>> Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1
>>>
>>> Fixes: e88975ca37d1 ("ath10k: dump tx stats in rate table format")
>>> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
>>
>> Fails to compile:
>>
>> drivers/net/wireless/ath/ath10k/htt_rx.c: In function
>> 'ath10k_accumulate_per_peer_tx_stats':
>> drivers/net/wireless/ath/ath10k/htt_rx.c:3252:14: error: implicit
>> declaration of function 'ath10k_get_bw'; did you mean
>> 'ath10k_get_tid'? [-Werror=implicit-function-declaration]
>> rtable_bw = ath10k_get_bw(&ar->hw_params, pstats->flags);
>> ^~~~~~~~~~~~~
>> ath10k_get_tid
>
> This set of patches is dependent on
> https://patchwork.kernel.org/patch/10831319/
> I can send out v2 for this patchset, marking the dependency, if needed.
Ok, I added this patchset back to my queue. No need to resend.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2] ath10k: add peer id check in ath10k_peer_find_by_id
From: Claire Chang @ 2019-05-03 7:19 UTC (permalink / raw)
To: Wen Gong; +Cc: ath10k, open list:NETWORKING DRIVERS (WIRELESS)
In-Reply-To: <1556536632-19433-1-git-send-email-wgong@codeaurora.org>
> Signed-off-by: Wen Gong <wgong@codeaurora.org>
Tested-by: Claire Chang <tientzu@chromium.org>
^ permalink raw reply
* [RFC PATCH 1/2] rtl8xxxu: Add rate adaptive related data
From: Chris Chiu @ 2019-05-03 7:21 UTC (permalink / raw)
To: jes.sorensen, kvalo, davem
Cc: linux-wireless, netdev, linux-kernel, linux, Chris Chiu
In-Reply-To: <20190503072146.49999-1-chiu@endlessm.com>
Add wireless mode, signal strength level, and rate table index
to tell the firmware that we need to adjust the tx rate bitmap
accordingly.
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 45 +++++++++++++++++++
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 45 ++++++++++++++++++-
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 8828baf26e7b..771f58aa7cae 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1195,6 +1195,50 @@ struct rtl8723bu_c2h {
struct rtl8xxxu_fileops;
+/*mlme related.*/
+enum wireless_mode {
+ WIRELESS_MODE_UNKNOWN = 0,
+ //Sub-Element
+ WIRELESS_MODE_B = BIT(0), // tx: cck only , rx: cck only, hw: cck
+ WIRELESS_MODE_G = BIT(1), // tx: ofdm only, rx: ofdm & cck, hw: cck & ofdm
+ WIRELESS_MODE_A = BIT(2), // tx: ofdm only, rx: ofdm only, hw: ofdm only
+ WIRELESS_MODE_N_24G = BIT(3), // tx: MCS only, rx: MCS & cck, hw: MCS & cck
+ WIRELESS_MODE_N_5G = BIT(4), // tx: MCS only, rx: MCS & ofdm, hw: ofdm only
+ WIRELESS_AUTO = BIT(5),
+ WIRELESS_MODE_AC = BIT(6),
+ WIRELESS_MODE_MAX = (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_A|WIRELESS_MODE_N_24G|WIRELESS_MODE_N_5G|WIRELESS_MODE_AC),
+};
+
+/* from rtlwifi/wifi.h */
+enum ratr_table_mode_new {
+ RATEID_IDX_BGN_40M_2SS = 0,
+ RATEID_IDX_BGN_40M_1SS = 1,
+ RATEID_IDX_BGN_20M_2SS_BN = 2,
+ RATEID_IDX_BGN_20M_1SS_BN = 3,
+ RATEID_IDX_GN_N2SS = 4,
+ RATEID_IDX_GN_N1SS = 5,
+ RATEID_IDX_BG = 6,
+ RATEID_IDX_G = 7,
+ RATEID_IDX_B = 8,
+ RATEID_IDX_VHT_2SS = 9,
+ RATEID_IDX_VHT_1SS = 10,
+ RATEID_IDX_MIX1 = 11,
+ RATEID_IDX_MIX2 = 12,
+ RATEID_IDX_VHT_3SS = 13,
+ RATEID_IDX_BGN_3SS = 14,
+};
+
+#define RTL8XXXU_RATR_STA_INIT 0
+#define RTL8XXXU_RATR_STA_HIGH 1
+#define RTL8XXXU_RATR_STA_MID 2
+#define RTL8XXXU_RATR_STA_LOW 3
+
+struct rtl8xxxu_rate_adaptive {
+ u16 wireless_mode;
+ u8 ratr_index;
+ u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */
+} __packed;
+
struct rtl8xxxu_priv {
struct ieee80211_hw *hw;
struct usb_device *udev;
@@ -1299,6 +1343,7 @@ struct rtl8xxxu_priv {
u8 pi_enabled:1;
u8 no_pape:1;
u8 int_buf[USB_INTR_CONTENT_LENGTH];
+ struct rtl8xxxu_rate_adaptive ra_info;
};
struct rtl8xxxu_rx_urb {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 039e5ca9d2e4..360e9bd837e5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4345,7 +4345,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
h2c.ramask.arg = 0x80;
- h2c.b_macid_cfg.data1 = 0;
+ h2c.b_macid_cfg.data1 = priv->ra_info.ratr_index;
if (sgi)
h2c.b_macid_cfg.data1 |= BIT(7);
@@ -4485,6 +4485,43 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx);
}
+static u16
+rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
+{
+ u16 network_type = WIRELESS_MODE_UNKNOWN;
+ u32 rate_mask;
+
+ rate_mask = (sta->supp_rates[0] & 0xfff) |
+ (sta->ht_cap.mcs.rx_mask[0] << 12) |
+ (sta->ht_cap.mcs.rx_mask[0] << 20);
+
+ if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ)
+ {
+ if (sta->vht_cap.vht_supported)
+ network_type = WIRELESS_MODE_AC;
+ else if (sta->ht_cap.ht_supported)
+ network_type = WIRELESS_MODE_N_5G;
+
+ network_type |= WIRELESS_MODE_A;
+ }
+ else
+ {
+ if (sta->vht_cap.vht_supported)
+ network_type = WIRELESS_MODE_AC;
+ else if (sta->ht_cap.ht_supported)
+ network_type = WIRELESS_MODE_N_24G;
+
+ if (sta->supp_rates[0] <= 0xf)
+ network_type |= WIRELESS_MODE_B;
+ else if (sta->supp_rates[0] & 0xf)
+ network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
+ else
+ network_type |= WIRELESS_MODE_G;
+ }
+
+ return network_type;
+}
+
static void
rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct ieee80211_bss_conf *bss_conf, u32 changed)
@@ -4503,6 +4540,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
if (bss_conf->assoc) {
u32 ramask;
int sgi = 0;
+ struct rtl8xxxu_rate_adaptive *ra;
rcu_read_lock();
sta = ieee80211_find_sta(vif, bss_conf->bssid);
@@ -4527,6 +4565,11 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
sgi = 1;
rcu_read_unlock();
+ ra = &priv->ra_info;
+ ra->wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
+ ra->ratr_index = RATEID_IDX_BGN_40M_2SS;
+ ra->rssi_level = RTL8XXXU_RATR_STA_INIT;
+
priv->fops->update_rate_mask(priv, ramask, sgi);
rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
--
2.21.0
^ permalink raw reply related
* [RFC PATCH 2/2] rtl8xxxu: Add watchdog to update rate mask by signal strength
From: Chris Chiu @ 2019-05-03 7:21 UTC (permalink / raw)
To: jes.sorensen, kvalo, davem
Cc: linux-wireless, netdev, linux-kernel, linux, Chris Chiu
In-Reply-To: <20190503072146.49999-1-chiu@endlessm.com>
Introduce watchdog to monitor signal then update the rate mask
accordingly. The rate mask update logic comes from the rtlwifi
refresh_rate_adaptive_mask() from different chips.
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 +
.../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 151 ++++++++++++++++++
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 38 +++++
3 files changed, 197 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 771f58aa7cae..f97271951053 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1239,6 +1239,11 @@ struct rtl8xxxu_rate_adaptive {
u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */
} __packed;
+struct rtl8xxxu_watchdog {
+ struct ieee80211_vif *vif;
+ struct delayed_work ra_wq;
+};
+
struct rtl8xxxu_priv {
struct ieee80211_hw *hw;
struct usb_device *udev;
@@ -1344,6 +1349,7 @@ struct rtl8xxxu_priv {
u8 no_pape:1;
u8 int_buf[USB_INTR_CONTENT_LENGTH];
struct rtl8xxxu_rate_adaptive ra_info;
+ struct rtl8xxxu_watchdog watchdog;
};
struct rtl8xxxu_rx_urb {
@@ -1380,6 +1386,8 @@ struct rtl8xxxu_fileops {
bool ht40);
void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
u32 ramask, int sgi);
+ void (*refresh_rate_mask) (struct rtl8xxxu_priv *priv, int signal,
+ struct ieee80211_sta *sta);
void (*report_connect) (struct rtl8xxxu_priv *priv,
u8 macid, bool connect);
void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index 26b674aca125..92c35afecae0 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1645,6 +1645,156 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
}
+static u8 rtl8723b_signal_to_rssi(int signal)
+{
+ if (signal < -95)
+ signal = -95;
+ return (u8)(signal + 95);
+}
+
+static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
+ int signal, struct ieee80211_sta *sta)
+{
+ struct rtl8xxxu_rate_adaptive *ra;
+ struct ieee80211_hw *hw = priv->hw;
+ u16 wireless_mode;
+ u8 rssi_level, ratr_index;
+ u8 txbw_40mhz;
+ u8 rssi, rssi_thresh_high, rssi_thresh_low;
+
+ ra = &priv->ra_info;
+ wireless_mode = ra->wireless_mode;
+ rssi_level = ra->rssi_level;
+ rssi = rtl8723b_signal_to_rssi(signal);
+ ratr_index = ra->ratr_index;
+ txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40)? 1 : 0;
+
+ switch (rssi_level) {
+ case RTL8XXXU_RATR_STA_HIGH:
+ rssi_thresh_high = 50;
+ rssi_thresh_low = 20;
+ break;
+ case RTL8XXXU_RATR_STA_MID:
+ rssi_thresh_high = 55;
+ rssi_thresh_low = 20;
+ break;
+ case RTL8XXXU_RATR_STA_LOW:
+ rssi_thresh_high = 60;
+ rssi_thresh_low = 25;
+ break;
+ default:
+ rssi_thresh_high = 50;
+ rssi_thresh_low = 20;
+ break;
+ }
+
+ if (rssi > rssi_thresh_high)
+ rssi_level = RTL8XXXU_RATR_STA_HIGH;
+ else if (rssi > rssi_thresh_low)
+ rssi_level = RTL8XXXU_RATR_STA_MID;
+ else
+ rssi_level = RTL8XXXU_RATR_STA_LOW;
+
+ if (rssi_level != ra->rssi_level) {
+ int sgi = 0;
+ u32 rate_bitmap = 0;
+
+ rcu_read_lock();
+ rate_bitmap = (sta->supp_rates[0] & 0xfff) |
+ sta->ht_cap.mcs.rx_mask[0] << 12 |
+ sta->ht_cap.mcs.rx_mask[1] << 20;
+ if (sta->ht_cap.cap &
+ (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
+ sgi = 1;
+ rcu_read_unlock();
+
+ switch (wireless_mode) {
+ case WIRELESS_MODE_B:
+ ratr_index = RATEID_IDX_B;
+ if (rate_bitmap & 0x0000000c)
+ rate_bitmap &= 0x0000000d;
+ else
+ rate_bitmap &= 0x0000000f;
+ break;
+ case WIRELESS_MODE_A:
+ case WIRELESS_MODE_G:
+ ratr_index = RATEID_IDX_G;
+ if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
+ rate_bitmap &= 0x00000f00;
+ else
+ rate_bitmap &= 0x00000ff0;
+ break;
+ case (WIRELESS_MODE_B|WIRELESS_MODE_G):
+ ratr_index = RATEID_IDX_BG;
+ if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
+ rate_bitmap &= 0x00000f00;
+ else if (rssi_level == RTL8XXXU_RATR_STA_MID)
+ rate_bitmap &= 0x00000ff0;
+ else
+ rate_bitmap &= 0x00000ff5;
+ break;
+ case WIRELESS_MODE_N_24G:
+ case WIRELESS_MODE_N_5G:
+ case (WIRELESS_MODE_G|WIRELESS_MODE_N_24G):
+ case (WIRELESS_MODE_A|WIRELESS_MODE_N_5G):
+ if (priv->tx_paths == 2 && priv->rx_paths == 2)
+ ratr_index = RATEID_IDX_GN_N2SS;
+ else
+ ratr_index = RATEID_IDX_GN_N1SS;
+ case (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_N_24G):
+ case (WIRELESS_MODE_B|WIRELESS_MODE_N_24G):
+ if (txbw_40mhz) {
+ if (priv->tx_paths == 2 && priv->rx_paths == 2)
+ ratr_index = RATEID_IDX_BGN_40M_2SS;
+ else
+ ratr_index = RATEID_IDX_BGN_40M_1SS;
+ }
+ else {
+ if (priv->tx_paths == 2 && priv->rx_paths == 2)
+ ratr_index = RATEID_IDX_BGN_20M_2SS_BN;
+ else
+ ratr_index = RATEID_IDX_BGN_20M_1SS_BN;
+ }
+
+ if (priv->tx_paths == 2 && priv->rx_paths == 2) {
+ if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
+ rate_bitmap &= 0x0f8f0000;
+ else if (rssi_level == RTL8XXXU_RATR_STA_MID)
+ rate_bitmap &= 0x0f8ff000;
+ else {
+ if (txbw_40mhz)
+ rate_bitmap &= 0x0f8ff015;
+ else
+ rate_bitmap &= 0x0f8ff005;
+ }
+ }
+ else {
+ if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
+ rate_bitmap &= 0x000f0000;
+ else if (rssi_level == RTL8XXXU_RATR_STA_MID)
+ rate_bitmap &= 0x000ff000;
+ else {
+ if (txbw_40mhz)
+ rate_bitmap &= 0x000ff015;
+ else
+ rate_bitmap &= 0x000ff005;
+ }
+ }
+ break;
+ default:
+ ratr_index = RATEID_IDX_BGN_40M_2SS;
+ rate_bitmap &= 0x0fffffff;
+ break;
+ }
+
+ ra->ratr_index = ratr_index;
+ ra->rssi_level = rssi_level;
+ priv->fops->update_rate_mask(priv, rate_bitmap, sgi);
+ }
+
+ return;
+}
+
struct rtl8xxxu_fileops rtl8723bu_fops = {
.parse_efuse = rtl8723bu_parse_efuse,
.load_firmware = rtl8723bu_load_firmware,
@@ -1665,6 +1815,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
.usb_quirks = rtl8xxxu_gen2_usb_quirks,
.set_tx_power = rtl8723b_set_tx_power,
.update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
+ .refresh_rate_mask = rtl8723b_refresh_rate_mask,
.report_connect = rtl8xxxu_gen2_report_connect,
.fill_txdesc = rtl8xxxu_fill_txdesc_v2,
.writeN_block_size = 1024,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 360e9bd837e5..8db479986e97 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4565,6 +4565,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
sgi = 1;
rcu_read_unlock();
+ priv->watchdog.vif = vif;
ra = &priv->ra_info;
ra->wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
ra->ratr_index = RATEID_IDX_BGN_40M_2SS;
@@ -5822,6 +5823,38 @@ rtl8xxxu_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
return 0;
}
+static void rtl8xxxu_watchdog_callback(struct work_struct *work)
+{
+ struct ieee80211_vif *vif;
+ struct rtl8xxxu_watchdog *wdog;
+ struct rtl8xxxu_priv *priv;
+
+ wdog = container_of(work, struct rtl8xxxu_watchdog, ra_wq.work);
+ priv = container_of(wdog, struct rtl8xxxu_priv, watchdog);
+ vif = wdog->vif;
+
+ if (vif) {
+ int signal;
+ struct ieee80211_sta *sta;
+
+ rcu_read_lock();
+ sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
+ if (!sta) {
+ struct device *dev = &priv->udev->dev;
+ dev_info(dev, "%s: no sta found\n", __func__);
+ rcu_read_unlock();
+ return;
+ }
+ rcu_read_unlock();
+
+ signal = ieee80211_ave_rssi(vif);
+ if (priv->fops->refresh_rate_mask)
+ priv->fops->refresh_rate_mask(priv, signal, sta);
+ }
+
+ schedule_delayed_work(&priv->watchdog.ra_wq, 2 * HZ);
+}
+
static int rtl8xxxu_start(struct ieee80211_hw *hw)
{
struct rtl8xxxu_priv *priv = hw->priv;
@@ -5878,6 +5911,8 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
}
+
+ schedule_delayed_work(&priv->watchdog.ra_wq, 2* HZ);
exit:
/*
* Accept all data and mgmt frames
@@ -6101,6 +6136,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
INIT_LIST_HEAD(&priv->rx_urb_pending_list);
spin_lock_init(&priv->rx_urb_lock);
INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
+ INIT_DELAYED_WORK(&priv->watchdog.ra_wq, rtl8xxxu_watchdog_callback);
usb_set_intfdata(interface, hw);
@@ -6226,6 +6262,8 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
mutex_destroy(&priv->usb_buf_mutex);
mutex_destroy(&priv->h2c_mutex);
+ cancel_delayed_work_sync(&priv->watchdog.ra_wq);
+
if (priv->udev->state != USB_STATE_NOTATTACHED) {
dev_info(&priv->udev->dev,
"Device still attached, trying to reset\n");
--
2.21.0
^ permalink raw reply related
* [RFC PATCH 0/2] Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Chris Chiu @ 2019-05-03 7:21 UTC (permalink / raw)
To: jes.sorensen, kvalo, davem
Cc: linux-wireless, netdev, linux-kernel, linux, Chris Chiu
We have 3 laptops which connect the wifi by the same RTL8723BU.
The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
They have the same problem with the in-kernel rtl8xxxu driver, the
iperf (as a client to an ethernet-connected server) gets ~1Mbps.
Nevertheless, the signal strength is reported as around -40dBm,
which is quite good. From the wireshark capture, the tx rate for each
data and qos data packet is only 1Mbps. Compare to the driver from
https://github.com/lwfinger/rtl8723bu, the same iperf test gets ~12
Mbps or more. The signal strength is reported similarly around
-40dBm. That's why we want to find out the cause and improve.
After reading the source code of the rtl8xxxu driver and Larry's, the
major difference is that Larry's driver has a watchdog which will keep
monitoring the signal quality and updating the rate mask just like the
rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
And this kind of watchdog also exists in rtlwifi driver of some specific
chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
the same member function named dm_watchdog and will invoke the
corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
mask.
Thus I created 2 commits and try to do the same thing on rtl8xxxu.
After applying these commits, the tx rate of each data and qos data
packet will be 39Mbps (MCS4) with the 0xf00000 as its tx rate mask.
The 20th bit ~ 23th bit means MCS4 to MCS7. It means that the firmware
still picks the lowest rate from the rate mask and explains why the
tx rate of data and qos data is always lowest 1Mbps because the default
rate mask passed is almost 0xFFFFFFF ranges from the basic CCK rate,
OFDM rate, and MCS rate. However, with Larry's driver, the tx rate
observed from wireshark under the same condition is almost 65Mbps or
72Mbps.
I believe the firmware of RTL8723BU may need fix. And I think we
can still bring in the dm_watchdog as rtlwifi to improve from the
driver side. Please leave precious comments for my commits and
suggest what I can do better. Or suggest if there's any better idea
to fix this. Thanks.
Chris Chiu (2):
rtl8xxxu: Add rate adaptive related data
rtl8xxxu: Add watchdog to update rate mask by signal strength
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 53 ++++++
.../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 151 ++++++++++++++++++
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 83 +++++++++-
3 files changed, 286 insertions(+), 1 deletion(-)
--
2.21.0
^ permalink raw reply
* Re: [RFC PATCH 1/2] rtl8xxxu: Add rate adaptive related data
From: Kalle Valo @ 2019-05-03 7:49 UTC (permalink / raw)
To: Chris Chiu
Cc: jes.sorensen, davem, linux-wireless, netdev, linux-kernel, linux
In-Reply-To: <20190503072146.49999-2-chiu@endlessm.com>
Chris Chiu <chiu@endlessm.com> writes:
> Add wireless mode, signal strength level, and rate table index
> to tell the firmware that we need to adjust the tx rate bitmap
> accordingly.
Please read Developer's Certificate of Origin and add Signed-off-by to
the commit logs:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#signed-off-by_missing
--
Kalle Valo
^ permalink raw reply
* [PATCH v3 02/10] dt-bindings: doc: reflect new NVMEM of_get_mac_address behaviour
From: Petr Štetiar @ 2019-05-03 7:55 UTC (permalink / raw)
To: netdev, devicetree, Rob Herring, Mark Rutland, Yisen Zhuang,
Salil Mehta, Masahiro Yamada, Kalle Valo, Matthias Brugger
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Frank Rowand,
Srinivas Kandagatla, Maxime Ripard, Petr Štetiar,
linux-kernel, linux-arm-kernel, linux-wireless, linux-mediatek
In-Reply-To: <1556870168-26864-1-git-send-email-ynezz@true.cz>
As of_get_mac_address now supports NVMEM under the hood, we need to update
the bindings documentation with the new nvmem-cell* properties, which would
mean copy&pasting a lot of redundant information to every binding
documentation currently referencing some of the MAC address properties.
So I've just removed all the references to the optional MAC address
properties and replaced them with the small note referencing
net/ethernet.txt file.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
Changes since v2:
* replaced only MAC address related optional properties with a text
referencing ethernet.txt
Documentation/devicetree/bindings/net/altera_tse.txt | 5 ++---
Documentation/devicetree/bindings/net/amd-xgbe.txt | 5 +++--
Documentation/devicetree/bindings/net/brcm,amac.txt | 4 ++--
Documentation/devicetree/bindings/net/cpsw.txt | 4 +++-
Documentation/devicetree/bindings/net/davinci_emac.txt | 5 +++--
Documentation/devicetree/bindings/net/dsa/dsa.txt | 5 ++---
Documentation/devicetree/bindings/net/ethernet.txt | 6 ++++--
Documentation/devicetree/bindings/net/hisilicon-femac.txt | 4 +++-
.../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt | 4 +++-
Documentation/devicetree/bindings/net/keystone-netcp.txt | 10 +++++-----
Documentation/devicetree/bindings/net/macb.txt | 5 ++---
Documentation/devicetree/bindings/net/marvell-pxa168.txt | 4 +++-
Documentation/devicetree/bindings/net/microchip,enc28j60.txt | 3 ++-
Documentation/devicetree/bindings/net/microchip,lan78xx.txt | 5 ++---
Documentation/devicetree/bindings/net/qca,qca7000.txt | 4 +++-
Documentation/devicetree/bindings/net/samsung-sxgbe.txt | 4 +++-
.../devicetree/bindings/net/snps,dwc-qos-ethernet.txt | 5 +++--
.../devicetree/bindings/net/socionext,uniphier-ave4.txt | 4 ++--
Documentation/devicetree/bindings/net/socionext-netsec.txt | 5 +++--
.../devicetree/bindings/net/wireless/mediatek,mt76.txt | 5 +++--
Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt | 4 ++--
21 files changed, 58 insertions(+), 42 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/altera_tse.txt b/Documentation/devicetree/bindings/net/altera_tse.txt
index 0e21df9..0b7d4d3 100644
--- a/Documentation/devicetree/bindings/net/altera_tse.txt
+++ b/Documentation/devicetree/bindings/net/altera_tse.txt
@@ -46,9 +46,8 @@ Required properties:
- reg: phy id used to communicate to phy.
- device_type: Must be "ethernet-phy".
-Optional properties:
-- local-mac-address: See ethernet.txt in the same directory.
-- max-frame-size: See ethernet.txt in the same directory.
+The MAC address will be determined using the optional properties defined in
+ethernet.txt.
Example:
diff --git a/Documentation/devicetree/bindings/net/amd-xgbe.txt b/Documentation/devicetree/bindings/net/amd-xgbe.txt
index 93dcb79..9c27dfc 100644
--- a/Documentation/devicetree/bindings/net/amd-xgbe.txt
+++ b/Documentation/devicetree/bindings/net/amd-xgbe.txt
@@ -24,8 +24,6 @@ Required properties:
- phy-mode: See ethernet.txt file in the same directory
Optional properties:
-- mac-address: mac address to be assigned to the device. Can be overridden
- by UEFI.
- dma-coherent: Present if dma operations are coherent
- amd,per-channel-interrupt: Indicates that Rx and Tx complete will generate
a unique interrupt for each DMA channel - this requires an additional
@@ -34,6 +32,9 @@ Optional properties:
0 - 1GbE and 10GbE (default)
1 - 2.5GbE and 10GbE
+The MAC address will be determined using the optional properties defined in
+ethernet.txt.
+
The following optional properties are represented by an array with each
value corresponding to a particular speed. The first array value represents
the setting for the 1GbE speed, the second value for the 2.5GbE speed and
diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
index 0bfad65..0120ebe 100644
--- a/Documentation/devicetree/bindings/net/brcm,amac.txt
+++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
@@ -16,8 +16,8 @@ Required properties:
registers (required for Northstar2)
- interrupts: Interrupt number
-Optional properties:
-- mac-address: See ethernet.txt file in the same directory
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Examples:
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 3264e19..7c7ac5e 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -49,10 +49,12 @@ Required properties:
Optional properties:
- dual_emac_res_vlan : Specifies VID to be used to segregate the ports
-- mac-address : See ethernet.txt file in the same directory
- phy_id : Specifies slave phy id (deprecated, use phy-handle)
- phy-handle : See ethernet.txt file in the same directory
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
+
Slave sub-nodes:
- fixed-link : See fixed-link.txt file in the same directory
diff --git a/Documentation/devicetree/bindings/net/davinci_emac.txt b/Documentation/devicetree/bindings/net/davinci_emac.txt
index ca83dcc..5e3579e 100644
--- a/Documentation/devicetree/bindings/net/davinci_emac.txt
+++ b/Documentation/devicetree/bindings/net/davinci_emac.txt
@@ -20,11 +20,12 @@ Required properties:
Optional properties:
- phy-handle: See ethernet.txt file in the same directory.
If absent, davinci_emac driver defaults to 100/FULL.
-- nvmem-cells: phandle, reference to an nvmem node for the MAC address
-- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used
- ti,davinci-rmii-en: 1 byte, 1 means use RMII
- ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM?
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
+
Example (enbw_cmc board):
eth0: emac@1e20000 {
compatible = "ti,davinci-dm6467-emac";
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
index d66a529..a237567 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
@@ -71,9 +71,8 @@ properties, described in binding documents:
Documentation/devicetree/bindings/net/fixed-link.txt
for details.
-- local-mac-address : See
- Documentation/devicetree/bindings/net/ethernet.txt
- for details.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Example
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index a686215..6992444 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -4,12 +4,14 @@ NOTE: All 'phy*' properties documented below are Ethernet specific. For the
generic PHY 'phys' property, see
Documentation/devicetree/bindings/phy/phy-bindings.txt.
-- local-mac-address: array of 6 bytes, specifies the MAC address that was
- assigned to the network device;
- mac-address: array of 6 bytes, specifies the MAC address that was last used by
the boot program; should be used in cases where the MAC address assigned to
the device by the boot program is different from the "local-mac-address"
property;
+- local-mac-address: array of 6 bytes, specifies the MAC address that was
+ assigned to the network device;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used
- max-speed: number, specifies maximum speed in Mbit/s supported by the device;
- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
the maximum frame size (there's contradiction in the Devicetree
diff --git a/Documentation/devicetree/bindings/net/hisilicon-femac.txt b/Documentation/devicetree/bindings/net/hisilicon-femac.txt
index d11af5e..5f96976 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-femac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-femac.txt
@@ -14,7 +14,6 @@ Required properties:
the PHY reset signal(optional).
- reset-names: should contain the reset signal name "mac"(required)
and "phy"(optional).
-- mac-address: see ethernet.txt [1].
- phy-mode: see ethernet.txt [1].
- phy-handle: see ethernet.txt [1].
- hisilicon,phy-reset-delays-us: triplet of delays if PHY reset signal given.
@@ -22,6 +21,9 @@ Required properties:
The 2nd cell is reset pulse in micro seconds.
The 3rd cell is reset post-delay in micro seconds.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt[1].
+
[1] Documentation/devicetree/bindings/net/ethernet.txt
Example:
diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
index eea73ad..cddf46b 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
@@ -18,7 +18,6 @@ Required properties:
- #size-cells: must be <0>.
- phy-mode: see ethernet.txt [1].
- phy-handle: see ethernet.txt [1].
-- mac-address: see ethernet.txt [1].
- clocks: clock phandle and specifier pair.
- clock-names: contain the clock name "mac_core"(required) and "mac_ifc"(optional).
- resets: should contain the phandle to the MAC core reset signal(optional),
@@ -31,6 +30,9 @@ Required properties:
The 2nd cell is reset pulse in micro seconds.
The 3rd cell is reset post-delay in micro seconds.
+The MAC address will be determined using the properties defined in
+ethernet.txt[1].
+
- PHY subnode: inherits from phy binding [2]
[1] Documentation/devicetree/bindings/net/ethernet.txt
diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
index 04ba1dc..3a65aab 100644
--- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -135,14 +135,14 @@ Optional properties:
are swapped. The netcp driver will swap the two DWORDs
back to the proper order when this property is set to 2
when it obtains the mac address from efuse.
-- local-mac-address: the driver is designed to use the of_get_mac_address api
- only if efuse-mac is 0. When efuse-mac is 0, the MAC
- address is obtained from local-mac-address. If this
- attribute is not present, then the driver will use a
- random MAC address.
- "netcp-device label": phandle to the device specification for each of NetCP
sub-module attached to this interface.
+The MAC address will be determined using the optional properties defined in
+ethernet.txt, as provided by the of_get_mac_address API and only if efuse-mac
+is set to 0. If any of the optional MAC address properties are not present,
+then the driver will use random MAC address.
+
Example binding:
netcp: netcp@2000000 {
diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 8b80515..9c5e944 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -26,9 +26,8 @@ Required properties:
Optional elements: 'tsu_clk'
- clocks: Phandles to input clocks.
-Optional properties:
-- nvmem-cells: phandle, reference to an nvmem node for the MAC address
-- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Optional properties for PHY child node:
- reset-gpios : Should specify the gpio for phy reset
diff --git a/Documentation/devicetree/bindings/net/marvell-pxa168.txt b/Documentation/devicetree/bindings/net/marvell-pxa168.txt
index 845a148..5574af3 100644
--- a/Documentation/devicetree/bindings/net/marvell-pxa168.txt
+++ b/Documentation/devicetree/bindings/net/marvell-pxa168.txt
@@ -11,7 +11,9 @@ Optional properties:
- #address-cells: must be 1 when using sub-nodes.
- #size-cells: must be 0 when using sub-nodes.
- phy-handle: see ethernet.txt file in the same directory.
-- local-mac-address: see ethernet.txt file in the same directory.
+
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Sub-nodes:
Each PHY can be represented as a sub-node. This is not mandatory.
diff --git a/Documentation/devicetree/bindings/net/microchip,enc28j60.txt b/Documentation/devicetree/bindings/net/microchip,enc28j60.txt
index 24626e0..a827592 100644
--- a/Documentation/devicetree/bindings/net/microchip,enc28j60.txt
+++ b/Documentation/devicetree/bindings/net/microchip,enc28j60.txt
@@ -21,8 +21,9 @@ Optional properties:
- spi-max-frequency: Maximum frequency of the SPI bus when accessing the ENC28J60.
According to the ENC28J80 datasheet, the chip allows a maximum of 20 MHz, however,
board designs may need to limit this value.
-- local-mac-address: See ethernet.txt in the same directory.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Example (for NXP i.MX28 with pin control stuff for GPIO irq):
diff --git a/Documentation/devicetree/bindings/net/microchip,lan78xx.txt b/Documentation/devicetree/bindings/net/microchip,lan78xx.txt
index 76786a0..11a6795 100644
--- a/Documentation/devicetree/bindings/net/microchip,lan78xx.txt
+++ b/Documentation/devicetree/bindings/net/microchip,lan78xx.txt
@@ -7,9 +7,8 @@ The Device Tree properties, if present, override the OTP and EEPROM.
Required properties:
- compatible: Should be one of "usb424,7800", "usb424,7801" or "usb424,7850".
-Optional properties:
-- local-mac-address: see ethernet.txt
-- mac-address: see ethernet.txt
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Optional properties of the embedded PHY:
- microchip,led-modes: a 0..4 element vector, with each element configuring
diff --git a/Documentation/devicetree/bindings/net/qca,qca7000.txt b/Documentation/devicetree/bindings/net/qca,qca7000.txt
index e4a8a51..21c36e5 100644
--- a/Documentation/devicetree/bindings/net/qca,qca7000.txt
+++ b/Documentation/devicetree/bindings/net/qca,qca7000.txt
@@ -23,7 +23,6 @@ Optional properties:
Numbers smaller than 1000000 or greater than 16000000
are invalid. Missing the property will set the SPI
frequency to 8000000 Hertz.
-- local-mac-address : see ./ethernet.txt
- qca,legacy-mode : Set the SPI data transfer of the QCA7000 to legacy mode.
In this mode the SPI master must toggle the chip select
between each data word. In burst mode these gaps aren't
@@ -31,6 +30,9 @@ Optional properties:
the QCA7000 is setup via GPIO pin strapping. If the
property is missing the driver defaults to burst mode.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
+
SPI Example:
/* Freescale i.MX28 SPI master*/
diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
index 46e5911..2cff6d8 100644
--- a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
+++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
@@ -21,10 +21,12 @@ Required properties:
range.
Optional properties:
-- mac-address: 6 bytes, mac address
- max-frame-size: Maximum Transfer Unit (IEEE defined MTU), rather
than the maximum frame size.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
+
Example:
aliases {
diff --git a/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt b/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
index 36f1aef..ad3c6e1 100644
--- a/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
+++ b/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
@@ -103,8 +103,6 @@ Required properties:
Optional properties:
- dma-coherent: Present if dma operations are coherent
-- mac-address: See ethernet.txt in the same directory
-- local-mac-address: See ethernet.txt in the same directory
- phy-reset-gpios: Phandle and specifier for any GPIO used to reset the PHY.
See ../gpio/gpio.txt.
- snps,en-lpi: If present it enables use of the AXI low-power interface
@@ -133,6 +131,9 @@ Optional properties:
- device_type: Must be "ethernet-phy".
- fixed-mode device tree subnode: see fixed-link.txt in the same directory
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
+
Examples:
ethernet2@40010000 {
clock-names = "phy_ref_clk", "apb_pclk";
diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
index fc8f017..4e85fc4 100644
--- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -31,8 +31,8 @@ Required properties:
- socionext,syscon-phy-mode: A phandle to syscon with one argument
that configures phy mode. The argument is the ID of MAC instance.
-Optional properties:
- - local-mac-address: See ethernet.txt in the same directory.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
Required subnode:
- mdio: A container for child nodes representing phy nodes.
diff --git a/Documentation/devicetree/bindings/net/socionext-netsec.txt b/Documentation/devicetree/bindings/net/socionext-netsec.txt
index 0cff94f..9d6c9feb 100644
--- a/Documentation/devicetree/bindings/net/socionext-netsec.txt
+++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt
@@ -26,11 +26,12 @@ Required properties:
Optional properties: (See ethernet.txt file in the same directory)
- dma-coherent: Boolean property, must only be present if memory
accesses performed by the device are cache coherent.
-- local-mac-address: See ethernet.txt in the same directory.
-- mac-address: See ethernet.txt in the same directory.
- max-speed: See ethernet.txt in the same directory.
- max-frame-size: See ethernet.txt in the same directory.
+The MAC address will be determined using the optional properties
+defined in ethernet.txt.
+
Example:
eth0: ethernet@522d0000 {
compatible = "socionext,synquacer-netsec";
diff --git a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
index 7b9a776..7466550 100644
--- a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
+++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
@@ -13,11 +13,12 @@ properties:
Optional properties:
-- mac-address: See ethernet.txt in the parent directory
-- local-mac-address: See ethernet.txt in the parent directory
- ieee80211-freq-limit: See ieee80211.txt
- mediatek,mtd-eeprom: Specify a MTD partition + offset containing EEPROM data
+The driver is using of_get_mac_address API, so the MAC address can be as well
+be set with corresponding optional properties defined in net/ethernet.txt.
+
Optional nodes:
- led: Properties for a connected LED
Optional properties:
diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
index b7396c8..aaaeeb5 100644
--- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -34,9 +34,9 @@ Optional properties:
ath9k wireless chip (in this case the calibration /
EEPROM data will be loaded from userspace using the
kernel firmware loader).
-- mac-address: See ethernet.txt in the parent directory
-- local-mac-address: See ethernet.txt in the parent directory
+The MAC address will be determined using the optional properties defined in
+net/ethernet.txt.
In this example, the node is defined as child node of the PCI controller:
&pci0 {
--
1.9.1
^ permalink raw reply related
* [PATCH v3 07/10] net: wireless: support of_get_mac_address new ERR_PTR error
From: Petr Štetiar @ 2019-05-03 7:56 UTC (permalink / raw)
To: netdev, devicetree, QCA ath9k Development, Kalle Valo,
Stanislaw Gruszka, Helmut Schaa, Matthias Brugger
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Frank Rowand,
Srinivas Kandagatla, Maxime Ripard, Petr Štetiar,
linux-wireless, linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <1556870168-26864-1-git-send-email-ynezz@true.cz>
There was NVMEM support added to of_get_mac_address, so it could now
return NULL and ERR_PTR encoded error values, so we need to adjust all
current users of of_get_mac_address to this new fact.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
drivers/net/wireless/ath/ath9k/init.c | 2 +-
drivers/net/wireless/mediatek/mt76/eeprom.c | 2 +-
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 98141b6..8be2da8 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -642,7 +642,7 @@ static int ath9k_of_init(struct ath_softc *sc)
}
mac = of_get_mac_address(np);
- if (mac)
+ if (!IS_ERR_OR_NULL(mac))
ether_addr_copy(common->macaddr, mac);
return 0;
diff --git a/drivers/net/wireless/mediatek/mt76/eeprom.c b/drivers/net/wireless/mediatek/mt76/eeprom.c
index a1529920d..7cb16ba 100644
--- a/drivers/net/wireless/mediatek/mt76/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt76/eeprom.c
@@ -94,7 +94,7 @@
return;
mac = of_get_mac_address(np);
- if (mac)
+ if (!IS_ERR_OR_NULL(mac))
memcpy(dev->macaddr, mac, ETH_ALEN);
#endif
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 357c094..ef52467 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -1007,7 +1007,7 @@ void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8 *eeprom_mac_addr
const char *mac_addr;
mac_addr = of_get_mac_address(rt2x00dev->dev->of_node);
- if (mac_addr)
+ if (!IS_ERR_OR_NULL(mac_addr))
ether_addr_copy(eeprom_mac_addr, mac_addr);
if (!is_valid_ether_addr(eeprom_mac_addr)) {
--
1.9.1
^ permalink raw reply related
* Re: [RFC 10/17] mt7615: mcu: remove skb_ret from mt7615_mcu_msg_send
From: Lorenzo Bianconi @ 2019-05-03 9:27 UTC (permalink / raw)
To: Roy Luo
Cc: nbd, lorenzo.bianconi, linux-wireless,
Ryder Lee (李庚諺)
In-Reply-To: <CA+zupgxPj_oziDiSkztE3zLxZ9BXMAbQ3Vb4eEWSJ4JrSE41=A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 13350 bytes --]
> On Thu, May 2, 2019 at 11:31 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> > > Hi Lorenzo,
> > >
> >
> > Hi Roy,
> >
> > > IMHO, the skb_ret parameter from mt7615_mcu_msg_send is not just used by
> > > patch semaphore control.
> > > The other cmds also got their own event response that can be interpret
> > > accordingly by their own caller, which can provide the status of the cmd
> > > (success/fail) and other information that might be helpful case by case.
> >
> >
> > Do you mean the fw does not put the status code in a 'event struct'
> > at the beginning of the skb data buffer? What are the cmds that do not use
> > that approach?
> >
> > Regards,
> > Lorenzo
> >
> >
> What is the 'event struct' you're referring to?
I mean the one defined in mt7615_mcu_patch_sem_ctrl:
struct event {
u8 status;
u8 reserved[3];
};
> The common part in the beginning of the skb should be mt7615_mcu_rxd, the
> payload after that varies by cmd.
> Most cmds put status code in the beginning of the payload, then append some
> cmd specific content after it.
> There is also a little difference between ext cmd and non-ext cmd in terms
> of mcu rxd, which is why we do this tricky -4 to get status code out of
> patch sem event.
> skb_pull(skb, sizeof(*rxd) - 4);
I guess in the future we can manage the mcu reply in mt7615_mcu_msg_send() or
even add a new routine for it run by mt7615_mcu_msg_send.
I would avoid to modify __mt76_mcu_send_msg signature since it is used even by
mt76x02 and mt7603 and most of the times we will pass NULL to it.
Regards,
Lorenzo
>
>
> Regards,
> Cheng-Hao (Roy) Luo
>
>
>
> > > For now, we just assume every cmd works well and do no error handling,
> > > which is risky.
> > > If we are to take FW response into consideration in the future, maybe we
> > > should keep this parameter.
> > >
> > >
> > > Regards,
> > > Cheng-Hao (Roy) Luo
> > >
> > >
> > > On Thu, May 2, 2019 at 12:08 AM Lorenzo Bianconi <lorenzo@kernel.org>
> > wrote:
> > >
> > > > Remove skb_ret parameter from mt7615_mcu_msg_send signature since it is
> > > > actually used just by mt7615_mcu_patch_sem_ctrl. This is a prelimanry
> > > > patch to use mt76 common mcu API
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > > .../net/wireless/mediatek/mt76/mt7615/mcu.c | 77 ++++++-------------
> > > > 1 file changed, 24 insertions(+), 53 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > > > b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > > > index b8d928e8949c..4d1d4c0bc2e2 100644
> > > > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > > > @@ -116,8 +116,7 @@ static int __mt7615_mcu_msg_send(struct mt7615_dev
> > > > *dev, struct sk_buff *skb,
> > > > }
> > > >
> > > > static int
> > > > -mt7615_mcu_msg_send(struct mt7615_dev *dev, struct sk_buff *skb,
> > > > - int cmd, struct sk_buff **skb_ret)
> > > > +mt7615_mcu_msg_send(struct mt7615_dev *dev, struct sk_buff *skb, int
> > cmd)
> > > > {
> > > > unsigned long expires = jiffies + 10 * HZ;
> > > > struct mt7615_mcu_rxd *rxd;
> > > > @@ -142,18 +141,11 @@ mt7615_mcu_msg_send(struct mt7615_dev *dev,
> > struct
> > > > sk_buff *skb,
> > > > if (seq != rxd->seq)
> > > > continue;
> > > >
> > > > - if (skb_ret) {
> > > > - int hdr_len = sizeof(*rxd);
> > > > -
> > > > - if (!test_bit(MT76_STATE_MCU_RUNNING,
> > > > - &dev->mt76.state))
> > > > - hdr_len -= 4;
> > > > - skb_pull(skb, hdr_len);
> > > > - *skb_ret = skb;
> > > > - } else {
> > > > - dev_kfree_skb(skb);
> > > > + if (cmd == -MCU_CMD_PATCH_SEM_CONTROL) {
> > > > + skb_pull(skb, sizeof(*rxd) - 4);
> > > > + ret = *skb->data;
> > > > }
> > > > -
> > > > + dev_kfree_skb(skb);
> > > > break;
> > > > }
> > > >
> > > > @@ -177,8 +169,7 @@ static int mt7615_mcu_init_download(struct
> > mt7615_dev
> > > > *dev, u32 addr,
> > > > };
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb,
> > > > -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
> > > > - NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > > > -MCU_CMD_TARGET_ADDRESS_LEN_REQ);
> > > > }
> > > >
> > > > static int mt7615_mcu_send_firmware(struct mt7615_dev *dev, const void
> > > > *data,
> > > > @@ -219,43 +210,26 @@ static int mt7615_mcu_start_firmware(struct
> > > > mt7615_dev *dev, u32 addr,
> > > > };
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ,
> > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ);
> > > > }
> > > >
> > > > static int mt7615_mcu_restart(struct mt7615_dev *dev)
> > > > {
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(NULL, 0);
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ);
> > > > }
> > > >
> > > > static int mt7615_mcu_patch_sem_ctrl(struct mt7615_dev *dev, bool get)
> > > > {
> > > > struct {
> > > > - __le32 operation;
> > > > + __le32 op;
> > > > } req = {
> > > > - .operation = cpu_to_le32(get ? PATCH_SEM_GET :
> > > > - PATCH_SEM_RELEASE),
> > > > + .op = cpu_to_le32(get ? PATCH_SEM_GET :
> > PATCH_SEM_RELEASE),
> > > > };
> > > > - struct event {
> > > > - u8 status;
> > > > - u8 reserved[3];
> > > > - } *resp;
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > > - struct sk_buff *skb_ret;
> > > > - int ret;
> > > >
> > > > - ret = mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_SEM_CONTROL,
> > > > - &skb_ret);
> > > > - if (ret)
> > > > - goto out;
> > > > -
> > > > - resp = (struct event *)(skb_ret->data);
> > > > - ret = resp->status;
> > > > - dev_kfree_skb(skb_ret);
> > > > -
> > > > -out:
> > > > - return ret;
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > -MCU_CMD_PATCH_SEM_CONTROL);
> > > > }
> > > >
> > > > static int mt7615_mcu_start_patch(struct mt7615_dev *dev)
> > > > @@ -268,7 +242,7 @@ static int mt7615_mcu_start_patch(struct mt7615_dev
> > > > *dev)
> > > > };
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_FINISH_REQ,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > -MCU_CMD_PATCH_FINISH_REQ);
> > > > }
> > > >
> > > > static int mt7615_driver_own(struct mt7615_dev *dev)
> > > > @@ -554,8 +528,7 @@ int mt7615_mcu_set_eeprom(struct mt7615_dev *dev)
> > > > for (off = MT_EE_NIC_CONF_0; off < __MT_EE_MAX; off++)
> > > > data[off - MT_EE_NIC_CONF_0].val = eep[off];
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_EFUSE_BUFFER_MODE,
> > > > - NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > > > MCU_EXT_CMD_EFUSE_BUFFER_MODE);
> > > > }
> > > >
> > > > int mt7615_mcu_init_mac(struct mt7615_dev *dev)
> > > > @@ -570,7 +543,7 @@ int mt7615_mcu_init_mac(struct mt7615_dev *dev)
> > > > };
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_MAC_INIT_CTRL,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_MAC_INIT_CTRL);
> > > > }
> > > >
> > > > int mt7615_mcu_set_rts_thresh(struct mt7615_dev *dev, u32 val)
> > > > @@ -589,7 +562,7 @@ int mt7615_mcu_set_rts_thresh(struct mt7615_dev
> > *dev,
> > > > u32 val)
> > > > };
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PROTECT_CTRL,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PROTECT_CTRL);
> > > > }
> > > >
> > > > int mt7615_mcu_set_wmm(struct mt7615_dev *dev, u8 queue,
> > > > @@ -627,7 +600,7 @@ int mt7615_mcu_set_wmm(struct mt7615_dev *dev, u8
> > > > queue,
> > > > }
> > > >
> > > > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EDCA_UPDATE,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EDCA_UPDATE);
> > > > }
> > > >
> > > > int mt7615_mcu_ctrl_pm_state(struct mt7615_dev *dev, int enter)
> > > > @@ -657,7 +630,7 @@ int mt7615_mcu_ctrl_pm_state(struct mt7615_dev
> > *dev,
> > > > int enter)
> > > > };
> > > > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PM_STATE_CTRL,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_PM_STATE_CTRL);
> > > > }
> > > >
> > > > static int __mt7615_mcu_set_dev_info(struct mt7615_dev *dev,
> > > > @@ -704,8 +677,7 @@ static int __mt7615_mcu_set_dev_info(struct
> > mt7615_dev
> > > > *dev,
> > > >
> > > > memcpy(skb_push(skb, sizeof(req_hdr)), &req_hdr,
> > sizeof(req_hdr));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_DEV_INFO_UPDATE,
> > > > - NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_DEV_INFO_UPDATE);
> > > > }
> > > >
> > > > int mt7615_mcu_set_dev_info(struct mt7615_dev *dev, struct
> > ieee80211_vif
> > > > *vif,
> > > > @@ -830,8 +802,7 @@ static int __mt7615_mcu_set_bss_info(struct
> > mt7615_dev
> > > > *dev,
> > > > bss_info_tag_handler[i].handler)
> > > > bss_info_tag_handler[i].handler(dev, bss_info,
> > > > skb);
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_BSS_INFO_UPDATE,
> > > > - NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_BSS_INFO_UPDATE);
> > > > }
> > > >
> > > > int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> > > > @@ -914,7 +885,7 @@ __mt7615_mcu_set_wtbl(struct mt7615_dev *dev, int
> > > > wlan_idx,
> > > > if (buf && buf_len)
> > > > memcpy(skb_put(skb, buf_len), buf, buf_len);
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_WTBL_UPDATE,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_WTBL_UPDATE);
> > > > }
> > > >
> > > > static enum mt7615_cipher_type
> > > > @@ -1092,7 +1063,7 @@ __mt7615_mcu_set_sta_rec(struct mt7615_dev *dev,
> > int
> > > > bss_idx,
> > > > if (buf && buf_len)
> > > > memcpy(skb_put(skb, buf_len), buf, buf_len);
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_STA_REC_UPDATE,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_STA_REC_UPDATE);
> > > > }
> > > >
> > > > int mt7615_mcu_set_sta_rec_bmc(struct mt7615_dev *dev,
> > > > @@ -1220,7 +1191,7 @@ int mt7615_mcu_set_bcn(struct mt7615_dev *dev,
> > > > struct ieee80211_vif *vif,
> > > >
> > > > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > >
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BCN_OFFLOAD,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BCN_OFFLOAD);
> > > > }
> > > >
> > > > int mt7615_mcu_set_channel(struct mt7615_dev *dev)
> > > > @@ -1285,12 +1256,12 @@ int mt7615_mcu_set_channel(struct mt7615_dev
> > *dev)
> > > > memset(req.txpower_sku, 0x3f, 49);
> > > >
> > > > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > > - ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH,
> > > > NULL);
> > > > + ret = mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_CHANNEL_SWITCH);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > > > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_RX_PATH,
> > > > NULL);
> > > > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_RX_PATH);
> > > > }
> > > >
> > > > int mt7615_mcu_set_ht_cap(struct mt7615_dev *dev, struct ieee80211_vif
> > > > *vif,
> > > > --
> > > > 2.20.1
> > > >
> > > >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
From: Yan-Hsuan Chuang <yhchuang@realtek.com>
Not to use while (1) to parse power sequence commands in an array.
Put the statement (when cmd is not NULL) instead to make the loop stop
when the next fetched command is NULL.
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index 25a923b..7487b2e 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -203,17 +203,14 @@ static int rtw_pwr_seq_parser(struct rtw_dev *rtwdev,
return -EINVAL;
}
- do {
- cmd = cmd_seq[idx];
- if (!cmd)
- break;
-
+ while ((cmd = cmd_seq[idx])) {
ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd);
if (ret)
return -EBUSY;
+ /* fetch next command */
idx++;
- } while (1);
+ };
return 0;
}
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox