Re: rtlwifi/rtl8192cu AP mode broken with PS STA
On 4/18/21 7:32 PM, Pkshih wrote: -Original Message- From: Maciej S. Szmigiero [mailto:m...@maciej.szmigiero.name] Sent: Sunday, April 18, 2021 2:08 AM To: Pkshih Cc: linux-wirel...@vger.kernel.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org; johan...@sipsolutions.net; kv...@codeaurora.org; Larry Finger Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA On 08.04.2021 21:04, Maciej S. Szmigiero wrote: On 08.04.2021 06:42, Pkshih wrote: -Original Message- From: Maciej S. Szmigiero [mailto:m...@maciej.szmigiero.name] Sent: Thursday, April 08, 2021 4:53 AM To: Larry Finger; Pkshih Cc: linux-wirel...@vger.kernel.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org; johan...@sipsolutions.net; kv...@codeaurora.org Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA (...) Maceij, Does this patch fix the problem? The beacon seems to be updating now and STAs no longer get stuck in PS mode. Although sometimes (every 2-3 minutes with continuous 1s interval pings) there is around 5s delay in updating the transmitted beacon - don't know why, maybe the NIC hardware still has the old version in queue? Since USB device doesn't update every beacon, dtim_count isn't updated neither. It leads STA doesn't awake properly. Please try to fix dtim_period=1 in hostapd.conf, which tells STA awakes every beacon interval. The situation is the same with dtim_period=1. (...) Ping-Ke, are you going to submit your set_tim() patch so at least the AP mode is usable with PS STAs or are you waiting for a solution to the delayed beacon update issue? I'm still trying to get a 8192cu, and then I can reproduce the symptom you met. However, I'm busy now; maybe I have free time two weeks later. Do you think I submit the set_tim() patch with your Reported-by and Tested-by first? PK, I would say yes. Get the fix in as soon as possible. Larry
Re: rtlwifi/rtl8192cu AP mode broken with PS STA
On 4/6/21 9:48 PM, Pkshih wrote: On Tue, 2021-04-06 at 11:25 -0500, Larry Finger wrote: On 4/6/21 7:06 AM, Maciej S. Szmigiero wrote: On 06.04.2021 12:00, Kalle Valo wrote: "Maciej S. Szmigiero" writes: On 29.03.2021 00:54, Maciej S. Szmigiero wrote: Hi, It looks like rtlwifi/rtl8192cu AP mode is broken when a STA is using PS, since the driver does not update its beacon to account for TIM changes, so a station that is sleeping will never learn that it has packets buffered at the AP. Looking at the code, the rtl8192cu driver implements neither the set_tim() callback, nor does it explicitly update beacon data periodically, so it has no way to learn that it had changed. This results in the AP mode being virtually unusable with STAs that do PS and don't allow for it to be disabled (IoT devices, mobile phones, etc.). I think the easiest fix here would be to implement set_tim() for example the way rt2x00 driver does: queue a work or schedule a tasklet to update the beacon data on the device. Are there any plans to fix this? The driver is listed as maintained by Ping-Ke. Yeah, power save is hard and I'm not surprised that there are drivers with broken power save mode support. If there's no fix available we should stop supporting AP mode in the driver. https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/api clearly documents that "For AP mode, it must (...) react to the set_tim() callback or fetch each beacon from mac80211". The driver isn't doing either so no wonder the beacon it is sending isn't getting updated. As I have said above, it seems to me that all that needs to be done here is to queue a work in a set_tim() callback, then call send_beacon_frame() from rtlwifi/core.c from this work. But I don't know the exact device semantics, maybe it needs some other notification that the beacon has changed, too, or even tries to manage the TIM bitmap by itself. It would be a shame to lose the AP mode for such minor thing, though. I would play with this myself, but unfortunately I don't have time to work on this right now. That's where my question to Realtek comes: are there plans to actually fix this? Yes, I am working on this. My only question is "if you are such an expert on the problem, why do you not fix it?" The example in rx200 is not particularly useful, and I have not found any other examples. Hi Larry, I have a draft patch that forks a work to do send_beacon_frame(), whose behavior like Maciej mentioned. I did test on RTL8821AE; it works well. But, it seems already work well even I don't apply this patch, and I'm still digging why. I don't have a rtl8192cu dongle on hand, but I'll try to find one. Maceij, Does this patch fix the problem? Larry
Re: rtlwifi/rtl8192cu AP mode broken with PS STA
On 4/6/21 7:06 AM, Maciej S. Szmigiero wrote: On 06.04.2021 12:00, Kalle Valo wrote: "Maciej S. Szmigiero" writes: On 29.03.2021 00:54, Maciej S. Szmigiero wrote: Hi, It looks like rtlwifi/rtl8192cu AP mode is broken when a STA is using PS, since the driver does not update its beacon to account for TIM changes, so a station that is sleeping will never learn that it has packets buffered at the AP. Looking at the code, the rtl8192cu driver implements neither the set_tim() callback, nor does it explicitly update beacon data periodically, so it has no way to learn that it had changed. This results in the AP mode being virtually unusable with STAs that do PS and don't allow for it to be disabled (IoT devices, mobile phones, etc.). I think the easiest fix here would be to implement set_tim() for example the way rt2x00 driver does: queue a work or schedule a tasklet to update the beacon data on the device. Are there any plans to fix this? The driver is listed as maintained by Ping-Ke. Yeah, power save is hard and I'm not surprised that there are drivers with broken power save mode support. If there's no fix available we should stop supporting AP mode in the driver. https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/api clearly documents that "For AP mode, it must (...) react to the set_tim() callback or fetch each beacon from mac80211". The driver isn't doing either so no wonder the beacon it is sending isn't getting updated. As I have said above, it seems to me that all that needs to be done here is to queue a work in a set_tim() callback, then call send_beacon_frame() from rtlwifi/core.c from this work. But I don't know the exact device semantics, maybe it needs some other notification that the beacon has changed, too, or even tries to manage the TIM bitmap by itself. It would be a shame to lose the AP mode for such minor thing, though. I would play with this myself, but unfortunately I don't have time to work on this right now. That's where my question to Realtek comes: are there plans to actually fix this? Yes, I am working on this. My only question is "if you are such an expert on the problem, why do you not fix it?" The example in rx200 is not particularly useful, and I have not found any other examples. Larry
Re: [PATCH] rtlwifi: Simplify locking of a skb list accesses
On 4/5/21 2:57 AM, Christophe JAILLET wrote:
The 'c2hcmd_lock' spinlock is only used to protect some __skb_queue_tail()
and __skb_dequeue() calls.
Use the lock provided in the skb itself and call skb_queue_tail() and
skb_dequeue(). These functions already include the correct locking.
Signed-off-by: Christophe JAILLET
---
drivers/net/wireless/realtek/rtlwifi/base.c | 15 ++-
drivers/net/wireless/realtek/rtlwifi/wifi.h | 1 -
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c
b/drivers/net/wireless/realtek/rtlwifi/base.c
index 6e8bd99e8911..2a7ee90a3f54 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -551,7 +551,6 @@ int rtl_init_core(struct ieee80211_hw *hw)
spin_lock_init(>locks.rf_lock);
spin_lock_init(>locks.waitq_lock);
spin_lock_init(>locks.entry_list_lock);
- spin_lock_init(>locks.c2hcmd_lock);
spin_lock_init(>locks.scan_list_lock);
spin_lock_init(>locks.cck_and_rw_pagea_lock);
spin_lock_init(>locks.fw_ps_lock);
@@ -2269,7 +2268,6 @@ static bool rtl_c2h_fast_cmd(struct ieee80211_hw *hw,
struct sk_buff *skb)
void rtl_c2hcmd_enqueue(struct ieee80211_hw *hw, struct sk_buff *skb)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
- unsigned long flags;
if (rtl_c2h_fast_cmd(hw, skb)) {
rtl_c2h_content_parsing(hw, skb);
@@ -2278,11 +2276,7 @@ void rtl_c2hcmd_enqueue(struct ieee80211_hw *hw, struct
sk_buff *skb)
}
/* enqueue */
- spin_lock_irqsave(>locks.c2hcmd_lock, flags);
-
- __skb_queue_tail(>c2hcmd_queue, skb);
-
- spin_unlock_irqrestore(>locks.c2hcmd_lock, flags);
+ skb_queue_tail(>c2hcmd_queue, skb);
/* wake up wq */
queue_delayed_work(rtlpriv->works.rtl_wq, >works.c2hcmd_wq, 0);
@@ -2340,16 +2334,11 @@ void rtl_c2hcmd_launcher(struct ieee80211_hw *hw, int
exec)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct sk_buff *skb;
- unsigned long flags;
int i;
for (i = 0; i < 200; i++) {
/* dequeue a task */
- spin_lock_irqsave(>locks.c2hcmd_lock, flags);
-
- skb = __skb_dequeue(>c2hcmd_queue);
-
- spin_unlock_irqrestore(>locks.c2hcmd_lock, flags);
+ skb = skb_dequeue(>c2hcmd_queue);
/* do it */
if (!skb)
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h
b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 9119144bb5a3..877ed6a1589f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -2450,7 +2450,6 @@ struct rtl_locks {
spinlock_t waitq_lock;
spinlock_t entry_list_lock;
spinlock_t usb_lock;
- spinlock_t c2hcmd_lock;
spinlock_t scan_list_lock; /* lock for the scan list */
/*FW clock change */
Acked-by: Larry Finger
Thanks,
Larry
Re: [PATCH] b43: N-PHY: Fix the update of coef for the PHY revision >= 3case
On 2/15/21 6:05 AM, Colin King wrote:
From: Colin Ian King
The documentation for the PHY update [1] states:
Loop 4 times with index i
If PHY Revision >= 3
Copy table[i] to coef[i]
Otherwise
Set coef[i] to 0
the copy of the table to coef is currently implemented the wrong way
around, table is being updated from uninitialized values in coeff.
Fix this by swapping the assignment around.
[1] https://bcm-v4.sipsolutions.net/802.11/PHY/N/RestoreCal/
Fixes: 2f258b74d13c ("b43: N-PHY: implement restoring general configuration")
Addresses-Coverity: ("Uninitialized scalar variable")
Signed-off-by: Colin Ian King
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c
b/drivers/net/wireless/broadcom/b43/phy_n.c
index b669dff24b6e..665b737fbb0d 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -5311,7 +5311,7 @@ static void b43_nphy_restore_cal(struct b43_wldev *dev)
for (i = 0; i < 4; i++) {
if (dev->phy.rev >= 3)
- table[i] = coef[i];
+ coef[i] = table[i];
else
coef[i] = 0;
}
Acked-by: Larry Finger
Good catch, thanks.
Larry
Re: [PATCH] staging: rtl8188eu: Add Edimax EW-7811UN V2 to device table
On 2/4/21 2:52 AM, Martin Kaiser wrote:
The Edimax EW-7811UN V2 uses an RTL8188EU chipset and works with this
driver.
Signed-off-by: Martin Kaiser
---
drivers/staging/rtl8188eu/os_dep/usb_intf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 43ebd11b53fe..efad43d8e465 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -41,6 +41,7 @@ static const struct usb_device_id rtw_usb_id_tbl[] = {
{USB_DEVICE(0x2357, 0x0111)}, /* TP-Link TL-WN727N v5.21 */
{USB_DEVICE(0x2C4E, 0x0102)}, /* MERCUSYS MW150US v2 */
{USB_DEVICE(0x0df6, 0x0076)}, /* Sitecom N150 v2 */
+ {USB_DEVICE(0x7392, 0xb811)}, /* Edimax EW-7811UN V2 */
{USB_DEVICE(USB_VENDER_ID_REALTEK, 0xffef)}, /* Rosewill RNX-N150NUB */
{} /* Terminating entry */
};
Acked-by: Larry Finger
Larry
Re: [PATCH] rtw88: 8821c: Add RFE 2 support
On 2/2/21 12:29 AM, Kalle Valo wrote: Kai-Heng Feng writes: On Wed, Aug 5, 2020 at 7:24 PM Kai-Heng Feng wrote: Hi Tony, On Aug 5, 2020, at 19:18, Tony Chuang wrote: 8821CE with RFE 2 isn't supported: [ 12.404834] rtw_8821ce :02:00.0: rfe 2 isn't supported [ 12.404937] rtw_8821ce :02:00.0: failed to setup chip efuse info [ 12.404939] rtw_8821ce :02:00.0: failed to setup chip information NACK The RFE type 2 should be working with some additional fixes. Did you tested connecting to AP with BT paired? No, I only tested WiFi. The antenna configuration is different with RFE type 0. I will ask someone else to fix them. Then the RFE type 2 modules can be supported. Good to know that, I'll be patient and wait for a real fix. It's been quite some time, is support for RFE type 2 ready now? It looks like this patch should add it: https://patchwork.kernel.org/project/linux-wireless/patch/20210202055012.8296-4-pks...@realtek.com/ New firmware (rtw8821c_fw.bin) is also needed. That is available at https://github.com/lwfinger/rtw88.git. Larry
Re: [PATCH] drivers: net: wireless: rtlwifi: fix bool comparison in expressions
On 1/8/21 9:32 AM, Aditya Srivastava wrote: There are certain conditional expressions in rtlwifi, where a boolean variable is compared with true/false, in forms such as (foo == true) or (false != bar), which does not comply with checkpatch.pl (CHECK: BOOL_COMPARISON), according to which boolean variables should be themselves used in the condition, rather than comparing with true/false E.g., in drivers/net/wireless/realtek/rtlwifi/ps.c, "if (find_p2p_ie == true)" can be replaced with "if (find_p2p_ie)" Replace all such expressions with the bool variables appropriately Signed-off-by: Aditya Srivastava --- - The changes made are compile tested - Applies perfecly on next-20210108 drivers/net/wireless/realtek/rtlwifi/ps.c | 4 ++-- drivers/net/wireless/realtek/rtlwifi/rtl8188ee/dm.c | 8 drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c | 4 ++-- drivers/net/wireless/realtek/rtlwifi/rtl8192c/dm_common.c | 4 ++-- drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c | 4 ++-- drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 8 6 files changed, 16 insertions(+), 16 deletions(-) As has been stated several times, this form of the subject is incorrect. It should be: "rtlwifi: : I would prefer that there be separate patches for each driver, not that the changes be lumped into a single patch as was done here. Such organization makes it a lot easier to find the patches for a given driver in case something goes wrong.Note: The driver for ps is rtl_pci, and that for rtl8192c is rtl8192c-common. The other driver names match their directory. Larry
Re: [PATCH] drivers: net: wireless: realtek: Fix the word association defautly de-faulty
On 1/5/21 5:55 AM, Joe Perches wrote: On Tue, 2021-01-05 at 17:11 +0530, Bhaskar Chowdhury wrote: On 22:24 Tue 05 Jan 2021, Julian Calaby wrote: Hi Bhaskar, [] and your change is just making this comment worse. really??? Not sure about it. I agree with Julian. I'm fairly sure it's worse. The change you suggest doesn't parse well and is extremely odd. If you _really_ want to just change this use (and the others), I repeat his suggestion of "by default". I agree with Julian and Joe. Your suggested change makes it worse! To match ALL previous commits/patches for these drivers, the subject should be "rtlwifi: : Fix description of usage of own bit in descriptor" For all drivers, that comment should be written as: /* By default, a beacon packet will only use the first * descriptor and the own bit may not be cleared by the hardware */ Larry
Re: [PATCH v2 1/4] rtlwifi: rtl8188ee: avoid accessing the data mapped to streaming DMA
On 11/17/20 7:53 PM, Jia-Ju Bai wrote:
In rtl88ee_tx_fill_cmddesc(), skb->data is mapped to streaming DMA on
line 677:
dma_addr_t mapping = dma_map_single(..., skb->data, ...);
On line 680, skb->data is assigned to hdr after cast:
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)(skb->data);
Then hdr->frame_control is accessed on line 681:
__le16 fc = hdr->frame_control;
This DMA access may cause data inconsistency between CPU and hardwre.
To fix this bug, hdr->frame_control is accessed before the DMA mapping.
Signed-off-by: Jia-Ju Bai
---
drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
What changed between v1 and v2?
As outlined in Documentation/process/submitting-patches.rst, you should add a
'---' marker and descrive what was changed. I usually summarize the changes, but
it is also possible to provide a diffstat of the changes as the above file shows.
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
index b9775eec4c54..c948dafa0c80 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
@@ -674,12 +674,12 @@ void rtl88ee_tx_fill_cmddesc(struct ieee80211_hw *hw,
u8 fw_queue = QSLT_BEACON;
__le32 *pdesc = (__le32 *)pdesc8;
- dma_addr_t mapping = dma_map_single(>pdev->dev, skb->data,
- skb->len, DMA_TO_DEVICE);
-
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)(skb->data);
__le16 fc = hdr->frame_control;
+ dma_addr_t mapping = dma_map_single(>pdev->dev, skb->data,
+ skb->len, DMA_TO_DEVICE);
+
if (dma_mapping_error(>pdev->dev, mapping)) {
rtl_dbg(rtlpriv, COMP_SEND, DBG_TRACE,
"DMA mapping error\n");
Re: [PATCH 05/41] rtl8192cu: trx: Demote clear abuse of kernel-doc format
On 11/2/20 5:23 AM, Lee Jones wrote: Fixes the following W=1 kernel build warning(s): drivers/net/wireless/realtek/rtlwifi/rtl8192cu/trx.c:455: warning: Function parameter or member 'txdesc' not described in '_rtl_tx_desc_checksum' Cc: Ping-Ke Shih Cc: Kalle Valo Cc: "David S. Miller" Cc: Jakub Kicinski Cc: Larry Finger Cc: linux-wirel...@vger.kernel.org Cc: net...@vger.kernel.org Signed-off-by: Lee Jones --- drivers/net/wireless/realtek/rtlwifi/rtl8192cu/trx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/trx.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/trx.c index 1ad0cf37f60bb..87f959d5d861d 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/trx.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/trx.c @@ -448,7 +448,7 @@ static void _rtl_fill_usb_tx_desc(__le32 *txdesc) set_tx_desc_first_seg(txdesc, 1); } -/** +/* *For HW recovery information */ static void _rtl_tx_desc_checksum(__le32 *txdesc) Did you check this patch with checkpatch.pl? I think you substituted one warning for another. The wireless-testing trees previously did not accept a bare "/*", which is why "/**" was present. This particular instance should have /* For HW recovery information */ as the comment. Larry
Re: [PATCH] ssb: Fix error return in ssb_bus_scan()
On 10/21/20 2:33 AM, Jing Xiangfeng wrote:
Fix to return error code -EINVAL from the error handling case instead
of 0.
Signed-off-by: Jing Xiangfeng
---
drivers/ssb/scan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ssb/scan.c b/drivers/ssb/scan.c
index f49ab1aa2149..4161e5d1f276 100644
--- a/drivers/ssb/scan.c
+++ b/drivers/ssb/scan.c
@@ -325,6 +325,7 @@ int ssb_bus_scan(struct ssb_bus *bus,
if (bus->nr_devices > ARRAY_SIZE(bus->devices)) {
pr_err("More than %d ssb cores found (%d)\n",
SSB_MAX_NR_CORES, bus->nr_devices);
+ err = -EINVAL;
goto err_unmap;
}
if (bus->bustype == SSB_BUSTYPE_SSB) {
You misread the code. The current version is returning -ENOMEM, not 0 for this
error. Returning -EINVAL could be regarded as as better value; however, this
error is not likely to appear and it does not make much difference!
In any case, the commit message is wrong. NACK.
Larry
Re: [PATCH -next 9/9] rtlwifi: rtl8723be: fix comparison to bool warning in hw.c
On 9/18/20 5:25 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c:861:6-35: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
index 3c7ba8214daf..0748aedce2ad 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
@@ -858,7 +858,7 @@ static bool _rtl8723be_init_mac(struct ieee80211_hw *hw)
rtl_write_word(rtlpriv, REG_CR, 0x2ff);
if (!rtlhal->mac_func_enable) {
- if (_rtl8723be_llt_table_init(hw) == false)
+ if (!_rtl8723be_llt_table_init(hw))
return false;
}
--
2.26.0.106.g9fadedd
Re: [PATCH -next 8/9] rtlwifi: rtl8192de: fix comparison to bool warning in hw.c
On 9/18/20 5:25 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c:566:14-20: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c:572:13-19: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c:581:14-20: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c:587:13-19: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
index 2deadc7339ce..f849291cc587 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
@@ -563,13 +563,13 @@ static bool _rtl92de_llt_table_init(struct ieee80211_hw
*hw)
/* 18. LLT_table_init(Adapter); */
for (i = 0; i < (txpktbuf_bndy - 1); i++) {
status = _rtl92de_llt_write(hw, i, i + 1);
- if (true != status)
+ if (!status)
return status;
}
/* end of list */
status = _rtl92de_llt_write(hw, (txpktbuf_bndy - 1), 0xFF);
- if (true != status)
+ if (!status)
return status;
/* Make the other pages as ring buffer */
@@ -578,13 +578,13 @@ static bool _rtl92de_llt_table_init(struct ieee80211_hw
*hw)
/* Otherwise used as local loopback buffer. */
for (i = txpktbuf_bndy; i < maxpage; i++) {
status = _rtl92de_llt_write(hw, i, (i + 1));
- if (true != status)
+ if (!status)
return status;
}
/* Let last entry point to the start entry of ring buffer */
status = _rtl92de_llt_write(hw, maxpage, txpktbuf_bndy);
- if (true != status)
+ if (!status)
return status;
return true;
--
2.26.0.106.g9fadedd
Re: [PATCH -next 7/9] rtlwifi: rtl8192ce: fix comparison to bool warning in hw.c
On 9/18/20 5:25 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c:616:14-20: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c:621:13-19: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c:626:14-20: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c:631:13-19: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
index d4cd186036fd..bb5a0c4aec93 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
@@ -613,22 +613,22 @@ static bool _rtl92ce_llt_table_init(struct ieee80211_hw
*hw)
for (i = 0; i < (txpktbuf_bndy - 1); i++) {
status = _rtl92ce_llt_write(hw, i, i + 1);
- if (true != status)
+ if (!status)
return status;
}
status = _rtl92ce_llt_write(hw, (txpktbuf_bndy - 1), 0xFF);
- if (true != status)
+ if (!status)
return status;
for (i = txpktbuf_bndy; i < maxpage; i++) {
status = _rtl92ce_llt_write(hw, i, (i + 1));
- if (true != status)
+ if (!status)
return status;
}
status = _rtl92ce_llt_write(hw, maxpage, txpktbuf_bndy);
- if (true != status)
+ if (!status)
return status;
return true;
--
2.26.0.106.g9fadedd
Re: [PATCH -next 5/9] rtlwifi: rtl8821ae: fix comparison to bool warning in phy.c
On 9/18/20 5:25 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c:1816:5-13: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c:1825:5-13: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c:1839:5-13: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
index 7832fae3d00f..38669b4d6190 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
@@ -1813,7 +1813,7 @@ static bool _rtl8821ae_phy_bb8821a_config_parafile(struct
ieee80211_hw *hw)
rtstatus = _rtl8821ae_phy_config_bb_with_headerfile(hw,
BASEBAND_CONFIG_PHY_REG);
- if (rtstatus != true) {
+ if (!rtstatus) {
pr_err("Write BB Reg Fail!!\n");
return false;
}
@@ -1822,7 +1822,7 @@ static bool _rtl8821ae_phy_bb8821a_config_parafile(struct
ieee80211_hw *hw)
rtstatus = _rtl8821ae_phy_config_bb_with_pgheaderfile(hw,
BASEBAND_CONFIG_PHY_REG);
}
- if (rtstatus != true) {
+ if (!rtstatus) {
pr_err("BB_PG Reg Fail!!\n");
return false;
}
@@ -1836,7 +1836,7 @@ static bool _rtl8821ae_phy_bb8821a_config_parafile(struct
ieee80211_hw *hw)
rtstatus = _rtl8821ae_phy_config_bb_with_headerfile(hw,
BASEBAND_CONFIG_AGC_TAB);
- if (rtstatus != true) {
+ if (!rtstatus) {
pr_err("AGC Table Fail\n");
return false;
}
--
2.26.0.106.g9fadedd
Re: [PATCH -next 6/9] rtlwifi: rtl8192cu: fix comparison to bool warning in hw.c
On 9/18/20 5:25 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c:831:14-49: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
index 3061bd81f39e..6312fddd9c00 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
@@ -828,7 +828,7 @@ static int _rtl92cu_init_mac(struct ieee80211_hw *hw)
? WMM_CHIP_B_TX_PAGE_BOUNDARY
: WMM_CHIP_A_TX_PAGE_BOUNDARY;
}
- if (false == rtl92c_init_llt_table(hw, boundary)) {
+ if (!rtl92c_init_llt_table(hw, boundary)) {
pr_err("Failed to init LLT Table!\n");
return -EINVAL;
}
--
2.26.0.106.g9fadedd
Re: [PATCH -next 4/9] rtlwifi: rtl8821ae: fix comparison to bool warning in hw.c
On 9/18/20 5:25 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c:1897:5-13: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index b2e5b9fda669..33ffc24d3675 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1894,7 +1894,7 @@ int rtl8821ae_hw_init(struct ieee80211_hw *hw)
}
rtstatus = _rtl8821ae_init_mac(hw);
- if (rtstatus != true) {
+ if (!rtstatus) {
pr_err("Init MAC failed\n");
err = 1;
return err;
--
2.26.0.106.g9fadedd
Re: [PATCH -next 3/9] rtlwifi: rtl8192cu: fix comparison to bool warning in mac.c
On 9/18/20 5:24 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c:161:14-17: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c:168:13-16: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c:179:14-17: WARNING:
Comparison to bool
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c:186:13-16: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
index d7afb6a186df..2890a495a23e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
@@ -158,14 +158,14 @@ bool rtl92c_init_llt_table(struct ieee80211_hw *hw, u32
boundary)
for (i = 0; i < (boundary - 1); i++) {
rst = rtl92c_llt_write(hw, i , i + 1);
- if (true != rst) {
+ if (!rst) {
pr_err("===> %s #1 fail\n", __func__);
return rst;
}
}
/* end of list */
rst = rtl92c_llt_write(hw, (boundary - 1), 0xFF);
- if (true != rst) {
+ if (!rst) {
pr_err("===> %s #2 fail\n", __func__);
return rst;
}
@@ -176,14 +176,14 @@ bool rtl92c_init_llt_table(struct ieee80211_hw *hw, u32
boundary)
*/
for (i = boundary; i < LLT_LAST_ENTRY_OF_TX_PKT_BUFFER; i++) {
rst = rtl92c_llt_write(hw, i, (i + 1));
- if (true != rst) {
+ if (!rst) {
pr_err("===> %s #3 fail\n", __func__);
return rst;
}
}
/* Let last entry point to the start entry of ring buffer */
rst = rtl92c_llt_write(hw, LLT_LAST_ENTRY_OF_TX_PKT_BUFFER, boundary);
- if (true != rst) {
+ if (!rst) {
pr_err("===> %s #4 fail\n", __func__);
return rst;
}
--
2.26.0.106.g9fadedd
Re: [PATCH -next 2/9] rtlwifi: rtl8192c: fix comparison to bool warning in phy_common.c
On 9/18/20 5:24 AM, Zheng Bin wrote:
Fixes coccicheck warning:
drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c:1106:14-18: WARNING:
Comparison to bool
Signed-off-by: Zheng Bin
---
drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Larry Finger
Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c
index fc6c81291cf5..6a3deca404b9 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c
@@ -1103,7 +1103,7 @@ static void _rtl92c_phy_path_adda_on(struct ieee80211_hw
*hw,
u32 i;
pathon = is_patha_on ? 0x04db25a4 : 0x0b1b25a4;
- if (false == is2t) {
+ if (!is2t) {
pathon = 0x0bdb25a0;
rtl_set_bbreg(hw, addareg[0], MASKDWORD, 0x0b1b25a0);
} else {
--
2.26.0.106.g9fadedd
Re: Warning on Kernel 5.9.0-rc1 on PowerBook G4 (ppc32), bisected to a5c3b9ffb0f4
On 8/31/20 5:46 AM, Anshuman Khandual wrote:
On 08/29/2020 06:40 AM, Larry Finger wrote:
In kernel 5.9.0-rc1 on a PowerBook G4 (ppc32), several warnings of the
following type are logged:
[ cut here ]
WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:185 set_pte_at+0x20/0x100
All those warnings triggered at the same place i.e
arch/powerpc/mm/pgtable.c:185 ?
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.9.0-rc2 #2
NIP: c002add4 LR: c07dba40 CTR:
REGS: f1019d70 TRAP: 0700 Not tainted (5.9.0-rc2)
MSR: 00029032 CR: 22000888 XER:
GPR00: c07dba40 f1019e28 eeca3220 eef7ace0 4e999000 eef7d664 f1019e50
GPR08: 007c2315 0001 007c2315 f1019e48 22000888 c00054dc
GPR16: 2ef7d000 07c2 fff0 eef7b000 04e8
eef7d000
GPR24: eef7c5c0 007c2315 4e999000 c05ef548 eef7d664 c087cda8
007c2315
NIP [c002add4] set_pte_at+0x20/0x100
LR [c07dba40] debug_vm_pgtable+0x29c/0x654
Call Trace:
[f1019e28] [c002b4ac] pte_fragment_alloc+0x24/0xe4 (unreliable)
[f1019e48] [c07dba40] debug_vm_pgtable+0x29c/0x654
[f1019e98] [c0005160] do_one_initcall+0x70/0x158
[f1019ef8] [c07c352c] kernel_init_freeable+0x1f4/0x1f8
[f1019f28] [c00054f0] kernel_init+0x14/0xfc
[f1019f38] [c001516c] ret_from_kernel_thread+0x14/0x1c
Instruction dump:
57ff053e 39610010 7c63fa14 4800308c 9421ffe0 7c0802a6 8125 bfa10014
7cbd2b78 90010024 552907fe 83e6 <0f09> 3d20c089 83c91280 813e0018
---[ end trace 4ef67686e5133716 ]---
Although the warnings do no harm, I suspect that they should be fixed in case
some future modification turns the warning statements into BUGS.
These warnings are from mm/debug_vm_pgtable.c test, wont be converted into BUGS.
But nonetheless, need to be addressed though.
The problem was bisected to commit a5c3b9ffb0f4 ("mm/debug_vm_pgtable: add tests
validating advanced arch page table helpers") by Anshuman Khandual
There are some known issues wrt DEBUG_VM_PGTABLE on certain ppc64 platforms. But
I thought it worked all right on ppc32 platforms though. Adding Christophe Leroy
here. Currently, there is a series under review that makes DEBUG_VM_PGTABLE work
correctly on ppc64 platforms. Could you please give it a try and see if it fixes
these warnings ?
https://patchwork.kernel.org/project/linux-mm/list/?series=339387
That series of patches did get rid of the warnings.
Thanks,
Larry
Warning on Kernel 5.9.0-rc1 on PowerBook G4 (ppc32), bisected to a5c3b9ffb0f4
In kernel 5.9.0-rc1 on a PowerBook G4 (ppc32), several warnings of the following
type are logged:
[ cut here ]
WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:185 set_pte_at+0x20/0x100
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.9.0-rc2 #2
NIP: c002add4 LR: c07dba40 CTR:
REGS: f1019d70 TRAP: 0700 Not tainted (5.9.0-rc2)
MSR: 00029032 CR: 22000888 XER:
GPR00: c07dba40 f1019e28 eeca3220 eef7ace0 4e999000 eef7d664 f1019e50
GPR08: 007c2315 0001 007c2315 f1019e48 22000888 c00054dc
GPR16: 2ef7d000 07c2 fff0 eef7b000 04e8
eef7d000
GPR24: eef7c5c0 007c2315 4e999000 c05ef548 eef7d664 c087cda8
007c2315
NIP [c002add4] set_pte_at+0x20/0x100
LR [c07dba40] debug_vm_pgtable+0x29c/0x654
Call Trace:
[f1019e28] [c002b4ac] pte_fragment_alloc+0x24/0xe4 (unreliable)
[f1019e48] [c07dba40] debug_vm_pgtable+0x29c/0x654
[f1019e98] [c0005160] do_one_initcall+0x70/0x158
[f1019ef8] [c07c352c] kernel_init_freeable+0x1f4/0x1f8
[f1019f28] [c00054f0] kernel_init+0x14/0xfc
[f1019f38] [c001516c] ret_from_kernel_thread+0x14/0x1c
Instruction dump:
57ff053e 39610010 7c63fa14 4800308c 9421ffe0 7c0802a6 8125 bfa10014
7cbd2b78 90010024 552907fe 83e6 <0f09> 3d20c089 83c91280 813e0018
---[ end trace 4ef67686e5133716 ]---
Although the warnings do no harm, I suspect that they should be fixed in case
some future modification turns the warning statements into BUGS.
The problem was bisected to commit a5c3b9ffb0f4 ("mm/debug_vm_pgtable: add tests
validating advanced arch page table helpers") by Anshuman Khandual
Thanks,
Larry
Re: [PATCH] rtlwifi: switch from 'pci_' to 'dma_' API
On 8/20/20 9:46 AM, Christophe JAILLET wrote: The wrappers in include/linux/pci-dma-compat.h should go away. The patch has been generated with the coccinelle script below and has been hand modified to replace GFP_ with a correct flag. It has been compile tested. The only file where some GFP_ flags are updated is 'pci.c'. When memory is allocated in '_rtl_pci_init_tx_ring()' and '_rtl_pci_init_rx_ring()' GFP_KERNEL can be used because both functions are called from a probe function and no spinlock is taken. The call chain is: rtl_pci_probe --> rtl_pci_init --> _rtl_pci_init_trx_ring --> _rtl_pci_init_rx_ring --> _rtl_pci_init_tx_ring @@ @@ -PCI_DMA_BIDIRECTIONAL +DMA_BIDIRECTIONAL @@ @@ -PCI_DMA_TODEVICE +DMA_TO_DEVICE @@ @@ -PCI_DMA_FROMDEVICE +DMA_FROM_DEVICE @@ @@ -PCI_DMA_NONE +DMA_NONE @@ expression e1, e2, e3; @@ -pci_alloc_consistent(e1, e2, e3) +dma_alloc_coherent(>dev, e2, e3, GFP_) @@ expression e1, e2, e3; @@ -pci_zalloc_consistent(e1, e2, e3) +dma_alloc_coherent(>dev, e2, e3, GFP_) @@ expression e1, e2, e3, e4; @@ -pci_free_consistent(e1, e2, e3, e4) +dma_free_coherent(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_map_single(e1, e2, e3, e4) +dma_map_single(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_unmap_single(e1, e2, e3, e4) +dma_unmap_single(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4, e5; @@ -pci_map_page(e1, e2, e3, e4, e5) +dma_map_page(>dev, e2, e3, e4, e5) @@ expression e1, e2, e3, e4; @@ -pci_unmap_page(e1, e2, e3, e4) +dma_unmap_page(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_map_sg(e1, e2, e3, e4) +dma_map_sg(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_unmap_sg(e1, e2, e3, e4) +dma_unmap_sg(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_single_for_cpu(e1, e2, e3, e4) +dma_sync_single_for_cpu(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_single_for_device(e1, e2, e3, e4) +dma_sync_single_for_device(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_sg_for_cpu(e1, e2, e3, e4) +dma_sync_sg_for_cpu(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_sg_for_device(e1, e2, e3, e4) +dma_sync_sg_for_device(>dev, e2, e3, e4) @@ expression e1, e2; @@ -pci_dma_mapping_error(e1, e2) +dma_mapping_error(>dev, e2) @@ expression e1, e2; @@ -pci_set_dma_mask(e1, e2) +dma_set_mask(>dev, e2) @@ expression e1, e2; @@ -pci_set_consistent_dma_mask(e1, e2) +dma_set_coherent_mask(>dev, e2) Signed-off-by: Christophe JAILLET --- If needed, see post from Christoph Hellwig on the kernel-janitors ML: https://marc.info/?l=kernel-janitors=158745678307186=4 --- drivers/net/wireless/realtek/rtlwifi/pci.c| 116 +- .../wireless/realtek/rtlwifi/rtl8188ee/hw.c | 9 +- .../wireless/realtek/rtlwifi/rtl8188ee/trx.c | 13 +- .../wireless/realtek/rtlwifi/rtl8192ce/trx.c | 14 +-- .../wireless/realtek/rtlwifi/rtl8192de/trx.c | 12 +- .../wireless/realtek/rtlwifi/rtl8192ee/trx.c | 13 +- .../wireless/realtek/rtlwifi/rtl8192se/trx.c | 12 +- .../wireless/realtek/rtlwifi/rtl8723ae/trx.c | 14 +-- .../wireless/realtek/rtlwifi/rtl8723be/hw.c | 9 +- .../wireless/realtek/rtlwifi/rtl8723be/trx.c | 13 +- .../wireless/realtek/rtlwifi/rtl8821ae/hw.c | 9 +- .../wireless/realtek/rtlwifi/rtl8821ae/trx.c | 13 +- 12 files changed, 115 insertions(+), 132 deletions(-) Tested-by: Larry Finger for rtl8821ae. Larry
Re: [PATCH 2/6] rtlwifi: Remove unnecessary parenthese in rtl_dbg uses
On 7/27/20 9:52 AM, Joe Perches wrote: On Mon, 2020-07-27 at 09:04 +, Pkshih wrote: So, I think you would like to have parenthesis intentionally. If so, test1 ? : (test2 ? :) would be better. If not, test1 ? : test2 ? : may be what you want (without any parenthesis). Use whatever style you like, it's unimportant to me and it's not worth spending any real time on it. If you are so busy, why did you jump in with patches that you knew I was already working on? You knew because you critiqued my first submission. @Kalle: Please drop my contributions in the sequence "PATCH v2 00/15] rtlwifi: Change RT_TRACE into rtl_dbg for all drivers". Larry
Re: [PATCH] staging: rtl8723bs: include: Fix coding style errors
On 7/26/20 3:40 AM, Aditya Jain wrote: On Sun, Jul 26, 2020 at 1:56 PM Greg KH wrote: On Sun, Jul 26, 2020 at 01:32:15PM +0530, Aditya Jain wrote: Fixing ERROR: "foo * bar" should be "foo *bar" in hal_phy_cfg.h as reported by checkpatch.pl Signed-off-by: Aditya Jain --- .../staging/rtl8723bs/include/hal_phy_cfg.h| 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/staging/rtl8723bs/include/hal_phy_cfg.h b/drivers/staging/rtl8723bs/include/hal_phy_cfg.h index 419ddb0733aa..fd5f377bad4f 100644 --- a/drivers/staging/rtl8723bs/include/hal_phy_cfg.h +++ b/drivers/staging/rtl8723bs/include/hal_phy_cfg.h @@ -42,7 +42,7 @@ u32 Data u32 PHY_QueryRFReg_8723B( -struct adapter * Adapter, +struct adapter *Adapter, u8 eRFPath, u32 RegAddr, u32 BitMask Ick, these are all horrid. How about just making these all on a single line like most functions have them instead of this one cleanup? Same for the other changes you made in this file. thanks, greg k-h Agreed. I'll clean it up. While you are at it, drop the "include;" from the subject. For staging, the usual subject is of the form "staging: driver: thing being done". In your case "staging: rtl8723bs: Fix coding style errors". The directory of the files are not relevant. I am also not in favor of the large white space between the variable type and the name, but that is probably the subject of separate patches. Larry
Re: [PATCH 1/1] STAGING - REALTEK RTL8188EU DRIVERS: Fix Coding Style Error
On 7/25/20 1:39 PM, Joe Perches wrote:
On Sat, 2020-07-25 at 12:47 -0500, Larry Finger wrote:
On 7/25/20 7:20 AM, Anant Thazhemadam wrote:
Running the checkpatch.pl script on the file for which patch was created, the
following error was found to exist.
ERROR: space required after that ',' (ctx:VxV)
Fixed the above error which was found on line #721 by inserting a blank
space at the appropriate position.
[]
diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c
b/drivers/staging/rtl8188eu/core/rtw_security.c
[]
@@ -718,7 +718,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8
*precvframe)
res = _FAIL;
}
} else {
- RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo==NULL!!!\n",__func__));
+ RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo==NULL!!!\n", __func__));
res = _FAIL;
}
}
In fixing one checkpatch.pl condition, you introduced another - the resulting
line is too long. You should fix only one such condition, but you should fix any
others that are introduced. You do need to document it.
I think that doesn't matter as it was also too long
before this change.
Patch subjects for this driver should be written as "staging: rtl8188eu: .".
How likely is it that this driver would ever be
moved to drivers/net/wireless/realtek/rtlwifi?
Very unlikely. It I wanted to undertake that kind of effort, I would switch to
one of the later versions from Realtek that uses nl80211/cfg80211. Despite that,
it is likely that only the USB driver from rtlwifi could be used.
Larry
Re: [PATCH 1/1] STAGING - REALTEK RTL8188EU DRIVERS: Fix Coding Style Error
On 7/25/20 7:20 AM, Anant Thazhemadam wrote:
Running the checkpatch.pl script on the file for which patch was created, the
following error was found to exist.
ERROR: space required after that ',' (ctx:VxV)
Fixed the above error which was found on line #721 by inserting a blank
space at the appropriate position.
Signed-off-by: Anant Thazhemadam
---
drivers/staging/rtl8188eu/core/rtw_security.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c
b/drivers/staging/rtl8188eu/core/rtw_security.c
index 21f6652dd69f..57e171d3e48d 100644
--- a/drivers/staging/rtl8188eu/core/rtw_security.c
+++ b/drivers/staging/rtl8188eu/core/rtw_security.c
@@ -718,7 +718,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8
*precvframe)
res = _FAIL;
}
} else {
- RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo==NULL!!!\n",__func__));
+ RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo==NULL!!!\n", __func__));
res = _FAIL;
}
}
In fixing one checkpatch.pl condition, you introduced another - the resulting
line is too long. You should fix only one such condition, but you should fix any
others that are introduced. You do need to document it.
Patch subjects for this driver should be written as "staging: rtl8188eu: .".
Larry
Re: [PATCH] Staging: rtl8188eu: rtw_mlme: Fix uninitialized variable authmode
On 7/24/20 8:28 AM, Dinghao Liu wrote:
The variable authmode will keep uninitialized if neither if
statements used to initialize this variable are not triggered.
Besides Greg's comment, you need to re-parse this sentence. I realize that
English is probably not your first language, but this one is not what you meant.
You likely meant "The variable authmode will remain uninitialized if all
statements used to initialize this variable are not triggered."
A possible (line-wrapped) patch to quiet the tools would be:
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 9de2d421f6b1..9e4d78bc9a2e 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1729,9 +1729,11 @@ int rtw_restruct_sec_ie(struct adapter *adapter, u8
*in_ie, u8 *out_ie, uint in_
if ((ndisauthmode == Ndis802_11AuthModeWPA) ||
(ndisauthmode == Ndis802_11AuthModeWPAPSK))
authmode = _WPA_IE_ID_;
- if ((ndisauthmode == Ndis802_11AuthModeWPA2) ||
- (ndisauthmode == Ndis802_11AuthModeWPA2PSK))
+ else if ((ndisauthmode == Ndis802_11AuthModeWPA2) ||
+(ndisauthmode == Ndis802_11AuthModeWPA2PSK))
authmode = _WPA2_IE_ID_;
+ else
+ authmode = 0;
if (check_fwstate(pmlmepriv, WIFI_UNDER_WPS)) {
memcpy(out_ie + ielength, psecuritypriv->wps_ie,
psecuritypriv->wps_ie_len);
Yes, in this routine, it would be possible for authmode to not be set; however,
later code only compares it to either _WPA_IE_ID_ or _WPA2_IE_ID_. It is never
used in a way that an unset value could make the program flow be different by
arbitrarily setting the value to zero. Thus your statement "Then authmode may
contain a garbage value and influence the execution flow of this function." is
false.
Larry
Re: [PATCH] staging: r8188eu: remove unused members of struct xmit_buf
On 7/13/20 1:28 PM, Ivan Safonov wrote:
On 7/13/20 5:23 PM, Dan Carpenter wrote:
On Mon, Jul 13, 2020 at 04:16:07PM +0300, Dan Carpenter wrote:
On Sun, Jul 12, 2020 at 03:38:21PM +0300, Ivan Safonov wrote:
Remove unused members of struct xmit_buf: alloc_sz, ff_hwaddr,
dma_transfer_addr, bpending and last.
Signed-off-by: Ivan Safonov
---
drivers/staging/rtl8188eu/include/rtw_xmit.h | 5 -
drivers/staging/rtl8188eu/os_dep/xmit_linux.c | 1 -
2 files changed, 6 deletions(-)
diff --git a/drivers/staging/rtl8188eu/include/rtw_xmit.h
b/drivers/staging/rtl8188eu/include/rtw_xmit.h
index 12d16e98176a..3c03987c81a1 100644
--- a/drivers/staging/rtl8188eu/include/rtw_xmit.h
+++ b/drivers/staging/rtl8188eu/include/rtw_xmit.h
@@ -193,14 +193,9 @@ struct xmit_buf {
void *priv_data;
u16 ext_tag; /* 0: Normal xmitbuf, 1: extension xmitbuf. */
u16 flags;
- u32 alloc_sz;
u32 len;
struct submit_ctx *sctx;
- u32 ff_hwaddr;
struct urb *pxmit_urb[8];
- dma_addr_t dma_transfer_addr; /* (in) dma addr for transfer_buffer */
- u8 bpending[8];
- int last[8];
};
struct xmit_frame {
diff --git a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
index 017e1d628461..61ced1160951 100644
--- a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
@@ -24,7 +24,6 @@ int rtw_os_xmit_resource_alloc(struct adapter *padapter,
return _FAIL;
pxmitbuf->pbuf = PTR_ALIGN(pxmitbuf->pallocated_buf, XMITBUF_ALIGN_SZ);
Not related to this patch but kmalloc always returns data which is at
least ARCH_KMALLOC_MINALIGN aligned which is never less than
XMITBUF_ALIGN_SZ (4) so this is a no-op.
4-byte alignment for 8-byte pointer (for example void *priv_data) on 64-bit arch
is an _error_. It’s good that kmalloc (and vmalloc) is already aligned to 8 bytes.
The alignment in the driver is pretty crazy because it's all unnecessary
and so complicated. Every allocation is 4 bytes extra so we can align
it later.
Also every buffer is called "pbuf" which stands for pointer to buffer.
"pallocated_buf" is not really useful either.
I tried to look at this to see if we could change the alignment, and
it's complicated because of the naming and the alignment.
regards,
dan carpenter
I have already fixed 4 places with unnecessary alignment, but, alas, there is no
great desire to test them on real hardware.
I have now tested on real hardware and it works fine.
Acked-by: Larry Finger
Larry
Re: [RFC PATCH 02/35] ssb: Change PCIBIOS_SUCCESSFUL to 0
On 7/13/20 2:13 PM, Saheed Bolarinwa wrote: Hello Larry, On 7/13/20 7:16 PM, Larry Finger wrote: On 7/13/20 7:22 AM, Saheed O. Bolarinwa wrote: In reference to the PCI spec (Chapter 2), PCIBIOS* is an x86 concept. Their scope should be limited within arch/x86. Change all PCIBIOS_SUCCESSFUL to 0 Signed-off-by: "Saheed O. Bolarinwa" Could you please tell me what difference this makes? It looks like source churn rather than a substantive change. The symbol is defined in pci.h and is used in many architures. Certainly, PCIBIOS_SUCCESSFUL indicates success even more clearly than 0 does. It is a trivial first step towards a probably significant task. I explained in the Cover Letter, I can see it didn't get through but I Cc linux-wireless (properly this time). Probably, too many addresses. I have resent it. It is here https://lore.kernel.org/linux-wireless/20200713185559.31967-1-refactormys...@gmail.com/T/#u Why is your name inside quotes in your s-o-b? To keep me company before I get to know my way within the kernel. I saw people with >2 names do it, so I did! Please let me know if it is odd. Thank you for the explanations. The cover letter did help. For both SSB and BMCA changes, Acked-by: Larry Finger Larry
Re: [RFC PATCH 02/35] ssb: Change PCIBIOS_SUCCESSFUL to 0
On 7/13/20 7:22 AM, Saheed O. Bolarinwa wrote: In reference to the PCI spec (Chapter 2), PCIBIOS* is an x86 concept. Their scope should be limited within arch/x86. Change all PCIBIOS_SUCCESSFUL to 0 Signed-off-by: "Saheed O. Bolarinwa" Could you please tell me what difference this makes? It looks like source churn rather than a substantive change. The symbol is defined in pci.h and is used in many architures. Certainly, PCIBIOS_SUCCESSFUL indicates success even more clearly than 0 does. Why is your name inside quotes in your s-o-b? Larry
Re: [PATCH] staging: r8188eu: remove unused members of struct xmit_buf
On 7/12/20 7:38 AM, Ivan Safonov wrote: Remove unused members of struct xmit_buf: alloc_sz, ff_hwaddr, dma_transfer_addr, bpending and last. Signed-off-by: Ivan Safonov --- Have you tested this change? Previously with this driver, an unused quantity was removed from one of the structs and the driver failed. Apparently, the alignment of some other quantity was affected. I do not think that this change would have that affect; however, you should be testing whenever the changes are more than cosmetic. Larry
Re: [PATCH] staging: rtl8712: switch to common ieee80211 headers
On 6/1/20 3:24 PM, Pascal Terjan wrote: This patch switches to and and deletes a lot of duplicate definitions plus many unused ones. Non obvious changes: - struct ieee80211_ht_cap is different enough that I preferred to keep (and rename) it for now. - mcs_rate in translate_scan was not read after being set, so I deleted that part rather than using the renamed struct - WLAN_CAPABILITY_BSS is replaced with WLAN_CAPABILITY_ESS which is the corresponding one with same value Signed-off-by: Pascal Terjan This patch does not apply to the staging repo, current mainline, or wireless-drivers-next. Where did you intend it to go? Staging is the correct tree. Larry
Re: Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
On 5/23/20 12:30 PM, Christophe Leroy wrote:
Hi Larry,
Le 23/05/2020 à 19:24, Larry Finger a écrit :
Hi Christophe,
Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4 when I
tried to generate a new kernel. The following BUG message is logged:
[...]
This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux PTE
bits to better match Hash PTE bits").
Being reversed in new -rc , see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/87sgfsf4hs@mpe.ellerman.id.au/
Christophe,
Thanks for the update.
Larry
Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
Hi Christophe,
Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4 when I
tried to generate a new kernel. The following BUG message is logged:
[ 336.148935] [ cut here ]
[ 336.148950] kernel BUG at ./include/linux/swapops.h:195!
[ 336.148971] Oops: Exception in kernel mode, sig: 5 [#1]
[ 336.148975] BE PAGE_SIZE=4K MMU=Hash PowerMac
[ 336.148978] Modules linked in: cpufreq_ondemand fuse ctr ccm b43 mac80211
sha256_generic libsha256 cfg80211 hid_apple hid_generic joydev rfkill libarc4
rng_core cordic uinput radeon evdev
ttm drm_kms_helper usbhid hid appletouch drm rack_meter ehci_pci ehci_hcd
drm_panel_orientation_quirks ssb fb_sys_fops yenta_socket sysimgblt sysfillrect
pcmcia_rsrc syscopyarea pcmcia pcmcia
_core i2c_powermac therm_adt746x loop ohci_pci ohci_hcd usbcore sungem
sungem_phy usb_common
[ 336.149052] CPU: 0 PID: 8346 Comm: ld Not tainted 5.6.0-rc2-00086-g36b7840
#249
[ 336.149056] NIP: c0166624 LR: c016661c CTR:
[ 336.149062] REGS: f42ddb08 TRAP: 0700 Not tainted
(5.6.0-rc2-00086-g36b7840)
[ 336.149064] MSR: 00029032 CR: 24000424 XER:
[ 336.149072]
[ 336.149072] GPR00: f42ddbc0 c24fcb80 0001 ef438f00 c1957b1c
f42ddc4c 0004
[ 336.149072] GPR08: 0050 0100 edb9d000 100cba68
10051b10 10051b08
[ 336.149072] GPR16: 01be ee68c078 105a c1957b1c
[ 336.149072] GPR24: ec5d2540 7c002bf8 c1957ae0 ec5d2540
ef438f00 ef438f00
[ 336.149107] NIP [c0166624] _einittext+0x3f9d38a8/0x3fe4a284
[ 336.149111] LR [c016661c] _einittext+0x3f9d38a0/0x3fe4a284
[ 336.149114] Call Trace:
[ 336.149118] [f42ddbc0] [c07b9b60] 0xc07b9b60 (unreliable)
[ 336.149123] [f42ddbd0] [c013ff64] _einittext+0x3f9ad1e8/0x3fe4a284
[ 336.149128] [f42ddc10] [c0140d4c] _einittext+0x3f9adfd0/0x3fe4a284
[ 336.149133] [f42ddc90] [c002aadc] _einittext+0x3f897d60/0x3fe4a284
[ 336.149137] [f42ddce0] [c00153a4] _einittext+0x3f882628/0x3fe4a284
[ 336.149144] --- interrupt: 301 at _einittext+0x3fb52a50/0x3fe4a284
[ 336.149144] LR = _einittext+0x3fb52a4c/0x3fe4a284
[ 336.149148] [f42ddda8] [c02e56c0] _einittext+0x3fb52944/0x3fe4a284
(unreliable)
[ 336.149153] [f42ddde8] [c011644c] _einittext+0x3f9836d0/0x3fe4a284
[ 336.149158] [f42dde38] [c01f5950] _einittext+0x3fa62bd4/0x3fe4a284
[ 336.149163] [f42dde58] [c016b98c] _einittext+0x3f9d8c10/0x3fe4a284
[ 336.149167] [f42ddec8] [c016ba60] _einittext+0x3f9d8ce4/0x3fe4a284
[ 336.149172] [f42ddef8] [c016bd00] _einittext+0x3f9d8f84/0x3fe4a284
[ 336.149177] [f42ddf38] [c0015174] _einittext+0x3f8823f8/0x3fe4a284
[ 336.149182] --- interrupt: c01 at 0xfdf99fc
[ 336.149182] LR = 0xfd9cce0
[ 336.149184] Instruction dump:
[ 336.149189] 40be0018 4bffe359 3c80c06a 3884e48f 4bfd4c9d 0fe0 4bffe345
7c641b78
[ 336.149196] 3860 4bffe045 7c630034 5463d97e <0f03> 3940 393f001c
3961
[ 336.149208] ---[ end trace d08833cae9c66ce3 ]---
[ 336.149210]
[ 336.193729] [ cut here ]
This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux PTE
bits to better match Hash PTE bits").
If I had done more rigorous tests earlier, I would have found the bug with more
time to fix it before release of 5.7, but every other problem I have found
happened at boot, not when the machine had to swap.
Thanks,
Larry
Indicated kmemleak in msr_build_context() for Kernel 5.7.0-rc5
On a system with an AMD FX-8320E Eight-Core Processor running kernel 5.7.0-rc5, I am seeing the following memory leak: localhost:~ # cat /sys/kernel/debug/kmemleak unreferenced object 0x88840ca02540 (size 64): comm "swapper/0", pid 1, jiffies 4294892775 (age 138786.084s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 04 10 01 c0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [<05004530>] msr_build_context.constprop.0+0x32/0xbe [] msr_save_cpuid_features+0x28/0x2c [<11ec0f08>] pm_check_save_msr+0x2e/0x40 [<0cd50945>] do_one_initcall+0x46/0x220 [ ] kernel_init_freeable+0x1c6/0x23f [<9f9b95ca>] kernel_init+0xa/0xfc [<0a571fca>] ret_from_fork+0x22/0x40 This is a "family 0x15" AMD CPU, thus MSR saving is needed during suspending. I believe this to be a false positive. The indicated memory allocation has been in the kernel since v4.5.0. Should a patch be sent to clear this false memory leak indication for systems with AMD processors? Larry
Re: [PATCH -next] net/wireless/rtl8225: Remove unused variable rtl8225z2_tx_power_ofdm
On 5/12/20 6:14 AM, ChenTao wrote:
Fix the following warning:
drivers/net/wireless/realtek/rtl818x/rtl8187/rtl8225.c:609:17: warning:
‘rtl8225z2_tx_power_ofdm’ defined but not used
static const u8 rtl8225z2_tx_power_ofdm[] = {
Reported-by: Hulk Robot
Signed-off-by: ChenTao
The patch is OK, but the subject is wrong. It should be "[PATCH-next] rtl8187:
Remove "
With that change, ACKed-by: Larry Finger
Larry
Re: Build failures since 5.4-rc3
On 10/15/19 2:32 PM, Joe Perches wrote: Hey Larry. Looks like this should be: #define FALL_THROUGH __attribute__((__fallthrough__)) and there appear to be many of these #defines that use __attribute__((foo)) where foo does not use the double underscored prefix and suffix form I also downloaded and trivially attempted to build vbox without success, but I don't find this #define anywhere in the sources. Clues? $ git clone git://github.com/mirror/vbox.git $ cd vbox $ git grep FALL_THROUGH $ $ ./configure Checking for environment: Determined build machine: linux.amd64, target machine: linux.amd64, OK. Checking for kBuild: ** kmk (variable KBUILDDIR) not found! Check /home/joe/vbox/configure.log for details $ cat configure.log # Log file generated by # # './configure ' # * Checking environment * Determined build machine: linux.amd64, target machine: linux.amd64 * Checking kBuild * ** kmk (variable KBUILDDIR) not found! $ I am the maintainer of VirtualBox for openSUSE, and it is their version that has the problem. The original code had the following macro definitions: # define RT_FALL_THROUGH() __attribute__((fallthrough)) #define RT_FALL_THRU() RT_FALL_THROUGH() The code uses both forms interchangeably. That failed - I think the () fooled the compiler. I replaced those with #define FALL_THROUGH __attribute__((__fallthrough__)) #define RT_FALL_THRU() FALL_THROUGH #define RT_FALL_THROUGH() FALL_THROUGH My initial try was without the underscores around fallthrough, which caused a conflict with the one in your changes. Putting them back resulted in code that builds fine. Thanks for the help. Larry
Build failures since 5.4-rc3
Joe,
Since commit 294f69e662d1("compiler_attributes.h: Add 'fallthrough' pseudo
keyword for switch/case use"), builds of VirtualBox are failing with the
following errors:
1954s] In file included from
/usr/src/linux-5.4.0-rc3-1.g2309d7d/include/linux/compiler_types.h:59,
[ 1954s] from :
[ 1954s]
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/SUPDrvGip.c:
In function 'supdrvTscDeltaThread':
[ 1954s]
/usr/src/linux-5.4.0-rc3-1.g2309d7d/include/linux/compiler_attributes.h:200:41:
error: expected ')' before '__attribute__'
[ 1954s] 200 | # define fallthrough
__attribute__((__fallthrough__))
[ 1954s] | ^
[ 1954s]
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/include/iprt/cdefs.h:1169:44:
note: in expansion of macro 'fallthrough'
[ 1954s] 1169 | # define FALL_THROUGH __attribute__ ((fallthrough))
[ 1954s] |^~~
[ 1954s]
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/include/iprt/cdefs.h:1176:33:
note: in expansion of macro 'FALL_THROUGH'
[ 1954s] 1176 | #define RT_FALL_THRU() FALL_THROUGH
[ 1954s] | ^~~~
[ 1954s]
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/SUPDrvGip.c:4192:17:
note: in expansion of macro 'RT_FALL_THRU'
[ 1954s] 4192 | RT_FALL_THRU();
[ 1954s] | ^~~~
[ 1954s] In file included from
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/include/VBox/cdefs.h:32,
[ 1954s] from
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/SUPDrvInternal.h:37,
[ 1954s] from
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/SUPDrvGip.c:33:
[ 1954s]
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/include/iprt/cdefs.h:1169:56:
error: expected identifier or '(' before ')' token
[ 1954s] 1169 | # define FALL_THROUGH __attribute__ ((fallthrough))
[ 1954s] |^
[ 1954s]
/home/abuild/rpmbuild/BUILD/VirtualBox-6.0.12/modules_build_dir/default/vboxdrv/include/iprt/cdefs.h:1176:33:
note: in expansion of macro 'FALL_THROUGH'
[ 1954s] 1176 | #define RT_FALL_THRU() FALL_THROUGH
[ 1954s] | ^~~~
I think the internal macros in the Oracle code are correct - at least they
worked before the patch in question was applied.
I would appreciate any suggestions.
Thanks,
Larry
Re: [PATCH v2] staging: rtl8188eu: remove dead code/vestigial do..while loop
On 9/24/19 9:28 AM, Connor Kuehl wrote:
The local variable 'bcmd_down' is always set to true almost immediately
before the do-while's condition is checked. As a result, !bcmd_down
evaluates to false which short circuits the logical AND operator meaning
that the second operand is never reached and is therefore dead code.
Furthermore, the do..while loop may be removed since it will always only
execute once because 'bcmd_down' is always set to true, so the
!bcmd_down evaluates to false and the loop exits immediately after the
first pass.
Fix this by removing the loop and its condition variables 'bcmd_down'
and 'retry_cnts'
While we're in there, also fix some checkpatch.pl suggestions regarding
spaces around arithmetic operators like '+'
Addresses-Coverity: ("Logically dead code")
Signed-off-by: Connor Kuehl
---
v1 -> v2:
- remove the loop and its condition variable bcmd_down
- address some non-invasive checkpatch.pl suggestions as a result of
deleting the loop
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 55 +---
1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 47352f210c0b..7646167a0b36 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -47,8 +47,6 @@ static u8 _is_fw_read_cmd_down(struct adapter *adapt, u8
msgbox_num)
**/
static s32 FillH2CCmd_88E(struct adapter *adapt, u8 ElementID, u32 CmdLen, u8
*pCmdBuffer)
{
- u8 bcmd_down = false;
- s32 retry_cnts = 100;
u8 h2c_box_num;
u32 msgbox_addr;
u32 msgbox_ex_addr;
@@ -71,39 +69,34 @@ static s32 FillH2CCmd_88E(struct adapter *adapt, u8
ElementID, u32 CmdLen, u8 *p
goto exit;
/* pay attention to if race condition happened in H2C cmd setting. */
- do {
- h2c_box_num = adapt->HalData->LastHMEBoxNum;
-
- if (!_is_fw_read_cmd_down(adapt, h2c_box_num)) {
- DBG_88E(" fw read cmd failed...\n");
- goto exit;
- }
-
- *(u8 *)(_cmd) = ElementID;
-
- if (CmdLen <= 3) {
- memcpy((u8 *)(_cmd)+1, pCmdBuffer, CmdLen);
- } else {
- memcpy((u8 *)(_cmd)+1, pCmdBuffer, 3);
- ext_cmd_len = CmdLen-3;
- memcpy((u8 *)(_cmd_ex), pCmdBuffer+3, ext_cmd_len);
+ h2c_box_num = adapt->HalData->LastHMEBoxNum;
- /* Write Ext command */
- msgbox_ex_addr = REG_HMEBOX_EXT_0 + (h2c_box_num *
RTL88E_EX_MESSAGE_BOX_SIZE);
- for (cmd_idx = 0; cmd_idx < ext_cmd_len; cmd_idx++)
- usb_write8(adapt, msgbox_ex_addr+cmd_idx, *((u8
*)(_cmd_ex)+cmd_idx));
- }
- /* Write command */
- msgbox_addr = REG_HMEBOX_0 + (h2c_box_num *
RTL88E_MESSAGE_BOX_SIZE);
- for (cmd_idx = 0; cmd_idx < RTL88E_MESSAGE_BOX_SIZE; cmd_idx++)
- usb_write8(adapt, msgbox_addr+cmd_idx, *((u8
*)(_cmd)+cmd_idx));
+ if (!_is_fw_read_cmd_down(adapt, h2c_box_num)) {
+ DBG_88E(" fw read cmd failed...\n");
+ goto exit;
+ }
- bcmd_down = true;
+ *(u8 *)(_cmd) = ElementID;
- adapt->HalData->LastHMEBoxNum =
- (h2c_box_num+1) % RTL88E_MAX_H2C_BOX_NUMS;
+ if (CmdLen <= 3) {
+ memcpy((u8 *)(_cmd) + 1, pCmdBuffer, CmdLen);
+ } else {
+ memcpy((u8 *)(_cmd) + 1, pCmdBuffer, 3);
+ ext_cmd_len = CmdLen - 3;
+ memcpy((u8 *)(_cmd_ex), pCmdBuffer + 3, ext_cmd_len);
+
+ /* Write Ext command */
+ msgbox_ex_addr = REG_HMEBOX_EXT_0 + (h2c_box_num *
RTL88E_EX_MESSAGE_BOX_SIZE);
+ for (cmd_idx = 0; cmd_idx < ext_cmd_len; cmd_idx++)
+ usb_write8(adapt, msgbox_ex_addr + cmd_idx, *((u8
*)(_cmd_ex) + cmd_idx));
+ }
+ /* Write command */
+ msgbox_addr = REG_HMEBOX_0 + (h2c_box_num * RTL88E_MESSAGE_BOX_SIZE);
+ for (cmd_idx = 0; cmd_idx < RTL88E_MESSAGE_BOX_SIZE; cmd_idx++)
+ usb_write8(adapt, msgbox_addr + cmd_idx, *((u8 *)(_cmd) +
cmd_idx));
- } while ((!bcmd_down) && (retry_cnts--));
+ adapt->HalData->LastHMEBoxNum =
+ (h2c_box_num + 1) % RTL88E_MAX_H2C_BOX_NUMS;
ret = _SUCCESS;
Acked-by: Larry Finger
Thanks,
Larry
Re: [PATCH] staging: rtl8188eu: fix HighestRate check in odm_ARFBRefresh_8188E()
On 9/26/19 2:31 AM, Denis Efremov wrote: It's incorrect to compare HighestRate with 0x0b twice in the following manner "if (HighestRate > 0x0b) ... else if (HighestRate > 0x0b) ...". The "else if" branch is constantly false. The second comparision should be with 0x03 according to the max_rate_idx in ODM_RAInfo_Init(). Cc: Larry Finger Cc: Greg Kroah-Hartman Cc: Michael Straube Cc: sta...@vger.kernel.org Signed-off-by: Denis Efremov --- drivers/staging/rtl8188eu/hal/hal8188e_rate_adaptive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/hal/hal8188e_rate_adaptive.c b/drivers/staging/rtl8188eu/hal/hal8188e_rate_adaptive.c index 9ddd51685063..5792f491b59a 100644 --- a/drivers/staging/rtl8188eu/hal/hal8188e_rate_adaptive.c +++ b/drivers/staging/rtl8188eu/hal/hal8188e_rate_adaptive.c @@ -409,7 +409,7 @@ static int odm_ARFBRefresh_8188E(struct odm_dm_struct *dm_odm, struct odm_ra_inf pRaInfo->PTModeSS = 3; else if (pRaInfo->HighestRate > 0x0b) pRaInfo->PTModeSS = 2; - else if (pRaInfo->HighestRate > 0x0b) + else if (pRaInfo->HighestRate > 0x03) pRaInfo->PTModeSS = 1; else pRaInfo->PTModeSS = 0; I agree that the original code is wrong; however, I prefer that changes that alter the execution should be tested. I see no evidence that such testing has been done. It probably does not matter because a highest rate between 3 and 0xb means 802.11g is in use, and that may no longer be a real-world situation. With any future patches, you need to indicate if testing has been done. Acked-by: Larry Finger Larry
Re: [PATCH v2] staging: rtl8188eu: fix possible null dereference
On 9/26/19 10:03 AM, Connor Kuehl wrote:
Inside a nested 'else' block at the beginning of this function is a
call that assigns 'psta' to the return value of 'rtw_get_stainfo()'.
If 'rtw_get_stainfo()' returns NULL and the flow of control reaches
the 'else if' where 'psta' is dereferenced, then we will dereference
a NULL pointer.
Fix this by checking if 'psta' is not NULL before reading its
'psta->qos_option' data member.
Addresses-Coverity: ("Dereference null return value")
Signed-off-by: Connor Kuehl
---
v1 -> v2:
- Add the same null check to line 779
drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 952f2ab51347..c37591657bac 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -776,7 +776,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
memcpy(pwlanhdr->addr2, get_bssid(pmlmepriv), ETH_ALEN);
memcpy(pwlanhdr->addr3, pattrib->src, ETH_ALEN);
- if (psta->qos_option)
+ if (psta && psta->qos_option)
qos_option = true;
} else if (check_fwstate(pmlmepriv, WIFI_ADHOC_STATE) ||
check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)) {
@@ -784,7 +784,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
memcpy(pwlanhdr->addr2, pattrib->src, ETH_ALEN);
memcpy(pwlanhdr->addr3, get_bssid(pmlmepriv), ETH_ALEN);
- if (psta->qos_option)
+ if (psta && psta->qos_option)
qos_option = true;
} else {
RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_, ("fw_state:%x
is not allowed to xmit frame\n", get_fwstate(pmlmepriv)));
Acked-by: Larry Finger
Thanks,
Larry
Re: [PATCH] rtlwifi: Remove excessive check in _rtl_ps_inactive_ps()
On 9/25/19 3:58 PM, Denis Efremov wrote:
There is no need to check "rtlhal->interface == INTF_PCI" twice in
_rtl_ps_inactive_ps(). The nested check is always true. Thus, the
expression can be simplified.
Signed-off-by: Denis Efremov
---
drivers/net/wireless/realtek/rtlwifi/ps.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/ps.c
b/drivers/net/wireless/realtek/rtlwifi/ps.c
index 70f04c2f5b17..6a8127539ea7 100644
--- a/drivers/net/wireless/realtek/rtlwifi/ps.c
+++ b/drivers/net/wireless/realtek/rtlwifi/ps.c
@@ -161,8 +161,7 @@ static void _rtl_ps_inactive_ps(struct ieee80211_hw *hw)
if (ppsc->inactive_pwrstate == ERFON &&
rtlhal->interface == INTF_PCI) {
if ((ppsc->reg_rfps_level & RT_RF_OFF_LEVL_ASPM) &&
- RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM) &&
- rtlhal->interface == INTF_PCI) {
+ RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
rtlpriv->intf_ops->disable_aspm(hw);
RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
}
Acked-by: Larry Finger
Thanks,
Larry
Re: [PATCH] staging: rtl8188eu: fix possible null dereference
On 9/25/19 4:32 PM, Connor Kuehl wrote:
Inside a nested 'else' block at the beginning of this function is a
call that assigns 'psta' to the return value of 'rtw_get_stainfo()'.
If 'rtw_get_stainfo()' returns NULL and the flow of control reaches
the 'else if' where 'psta' is dereferenced, then we will dereference
a NULL pointer.
Fix this by checking if 'psta' is not NULL before reading its
'psta->qos_option' data member.
Addresses-Coverity: ("Dereference null return value")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/core/rtw_xmit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 952f2ab51347..bf8877cbe9b6 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -784,7 +784,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
memcpy(pwlanhdr->addr2, pattrib->src, ETH_ALEN);
memcpy(pwlanhdr->addr3, get_bssid(pmlmepriv), ETH_ALEN);
- if (psta->qos_option)
+ if (psta && psta->qos_option)
qos_option = true;
} else {
RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_, ("fw_state:%x
is not allowed to xmit frame\n", get_fwstate(pmlmepriv)));
This change is a good one, but why not get the same fix at line 779?
Larry
Re: [PATCH] staging: rtl8188eu: remove dead code in do-while conditional step
On 9/23/19 2:48 PM, Connor Kuehl wrote:
The local variable 'bcmd_down' is always set to true almost immediately
before the do-while's condition is checked. As a result, !bcmd_down
evaluates to false which short circuits the logical AND operator meaning
that the second operand is never reached and is therefore dead code.
Addresses-Coverity: ("Logically dead code")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 47352f210c0b..a4b317937b23 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -48,7 +48,6 @@ static u8 _is_fw_read_cmd_down(struct adapter *adapt, u8
msgbox_num)
static s32 FillH2CCmd_88E(struct adapter *adapt, u8 ElementID, u32 CmdLen, u8
*pCmdBuffer)
{
u8 bcmd_down = false;
- s32 retry_cnts = 100;
u8 h2c_box_num;
u32 msgbox_addr;
u32 msgbox_ex_addr;
@@ -103,7 +102,7 @@ static s32 FillH2CCmd_88E(struct adapter *adapt, u8
ElementID, u32 CmdLen, u8 *p
adapt->HalData->LastHMEBoxNum =
(h2c_box_num+1) % RTL88E_MAX_H2C_BOX_NUMS;
- } while ((!bcmd_down) && (retry_cnts--));
+ } while (!bcmd_down);
ret = _SUCCESS;
This patch is correct; however, the do..while loop will always be executed once,
thus you could remove the loop and the loop variable bcmd_down.
@greg: If you would prefer a two-step process, then this one is OK.
Larry
Re: [PATCH] x86/mm: Remove set_pages_x() and set_pages_nx()
On 9/18/19 12:53 PM, Linus Torvalds wrote: On Wed, Sep 18, 2019 at 10:50 AM Larry Finger wrote: Is there approved way for pages to be set to be executable by an external module that would not be a security issue? Point to what external module and why. Honestly, the likely answer is simply "no". Why would an external module ever need to make something executable that isn't read-only code? That's pretty fundamental. Marking data executable is fairly questionable these days. Instead, what might work is to have some higher-level concept that we actually trust, and that isn't about making data executable, but about doing something reasonable. See the difference? Making things executable is not ok, but perhaps a "alternative runtime code sequence" is ok. Linus Linus, Yes, I do see the difference. The external module is vboxdrv, which is part of VirtualBox. The setting of pages to be executable appears to have been added in kernel 2.4.20. I am now testing with the former calls to set_pages_x() and set_pages_nx() disabled. Thus far, VMs seem to be running OK. I will contact Oracle to discuss the matter with them and see if there is some special case that requires this facility. If there is one, then they will need to discuss it with you and Christoph. Larry
Re: [PATCH] x86/mm: Remove set_pages_x() and set_pages_nx()
On 9/18/19 11:45 AM, Christoph Hellwig wrote:
On Wed, Sep 18, 2019 at 11:41:21AM -0500, Larry Finger wrote:
In commit 185be15143aa ("x86/mm: Remove set_pages_x() and set_pages_nx()"),
the wrappers were removed as they did not provide a real benefit over
set_memory_x() and set_memory_nx(). This change causes a problem because
the wrappers were exported, but the underlying routines were not. As a
result, external modules that used the wrappers would need to recreate
a significant part of memory management.
And external modules do not matter for mainline decisions. In fact
ensuring random modules can't mess with the NX state was one of the
reasons for this patch, as that is a security issue waiting to happen.
Christoph,
Is there approved way for pages to be set to be executable by an external module
that would not be a security issue?
Larry
[PATCH v2] x86/mm: Fix 185be15143aa ("Remove set_pages_x() and set_pages_nx()")
In commit 185be15143aa ("x86/mm: Remove set_pages_x() and set_pages_nx()"),
the wrappers were removed as they did not provide a real benefit over
set_memory_x() and set_memory_nx(). This change causes a problem because
the wrappers were exported, but the underlying routines were not. As a
result, external modules that used the wrappers would need to recreate
a significant part of memory management.
Signed-off-by: Larry Finger
Cc: Christoph Hellwig
Cc: Peter Zijlstra (Intel)
Fixes: 185be15143aa ("x86/mm: Remove set_pages_x() and set_pages_nx()")
Cc: Ingo Molnar
---
v2 - Subject was messed up
---
arch/x86/mm/pageattr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 0d09cc5aad61..755867fc7c19 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1885,6 +1885,7 @@ int set_memory_x(unsigned long addr, int numpages)
return change_page_attr_clear(, numpages, __pgprot(_PAGE_NX), 0);
}
+EXPORT_SYMBOL(set_memory_x);
int set_memory_nx(unsigned long addr, int numpages)
{
@@ -1893,6 +1894,7 @@ int set_memory_nx(unsigned long addr, int numpages)
return change_page_attr_set(, numpages, __pgprot(_PAGE_NX), 0);
}
+EXPORT_SYMBOL(set_memory_nx);
int set_memory_ro(unsigned long addr, int numpages)
{
--
2.23.0
[PATCH] x86/mm: Remove set_pages_x() and set_pages_nx()
In commit 185be15143aa ("x86/mm: Remove set_pages_x() and set_pages_nx()"),
the wrappers were removed as they did not provide a real benefit over
set_memory_x() and set_memory_nx(). This change causes a problem because
the wrappers were exported, but the underlying routines were not. As a
result, external modules that used the wrappers would need to recreate
a significant part of memory management.
Signed-off-by: Larry Finger
Cc: Christoph Hellwig
Cc: Peter Zijlstra (Intel)
Fixes: 185be15143aa ("x86/mm: Remove set_pages_x() and set_pages_nx()")
Cc: Ingo Molnar
---
arch/x86/mm/pageattr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 0d09cc5aad61..755867fc7c19 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1885,6 +1885,7 @@ int set_memory_x(unsigned long addr, int numpages)
return change_page_attr_clear(, numpages, __pgprot(_PAGE_NX), 0);
}
+EXPORT_SYMBOL(set_memory_x);
int set_memory_nx(unsigned long addr, int numpages)
{
@@ -1893,6 +1894,7 @@ int set_memory_nx(unsigned long addr, int numpages)
return change_page_attr_set(, numpages, __pgprot(_PAGE_NX), 0);
}
+EXPORT_SYMBOL(set_memory_nx);
int set_memory_ro(unsigned long addr, int numpages)
{
--
2.23.0
Re: [PATCH] staging: rtl8188eu: make two arrays static const, makes object smaller
On 9/6/19 12:39 PM, Colin King wrote: From: Colin Ian King Don't populate two arrays on the stack but instead make them static const. Makes the object code smaller by 49 bytes. Before: text data bss dec hex filename 26821 5616 0 324377eb5 rtl8188eu/core/rtw_ieee80211.o After: text data bss dec hex filename 26612 5776 0 323887e84 rtl8188eu/core/rtw_ieee80211.o (gcc version 9.2.1, amd64) Signed-off-by: Colin Ian King --- Acked-by: Larry Finger Thanks, Larry
Re: [PATCH] bcma: fix incorrect update of BCMA_CORE_PCI_MDIO_DATA
On 8/22/19 11:11 AM, Colin Ian King wrote:
On 22/08/2019 17:03, Larry Finger wrote:
On 8/22/19 8:35 AM, Colin King wrote:
From: Colin Ian King
An earlier commit re-worked the setting of the bitmask and is now
assigning v with some bit flags rather than bitwise or-ing them
into v, consequently the earlier bit-settings of v are being lost.
Fix this by replacing an assignment with the bitwise or instead.
Addresses-Coverity: ("Unused value")
Fixes: 2be25cac8402 ("bcma: add constants for PCI and use them")
Signed-off-by: Colin Ian King
---
drivers/bcma/driver_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bcma/driver_pci.c b/drivers/bcma/driver_pci.c
index f499a469e66d..d219ee947c07 100644
--- a/drivers/bcma/driver_pci.c
+++ b/drivers/bcma/driver_pci.c
@@ -78,7 +78,7 @@ static u16 bcma_pcie_mdio_read(struct bcma_drv_pci
*pc, u16 device, u8 address)
v |= (address << BCMA_CORE_PCI_MDIODATA_REGADDR_SHF_OLD);
}
- v = BCMA_CORE_PCI_MDIODATA_START;
+ v |= BCMA_CORE_PCI_MDIODATA_START;
v |= BCMA_CORE_PCI_MDIODATA_READ;
v |= BCMA_CORE_PCI_MDIODATA_TA;
I'm not sure the "Fixes" attribute is correct.
The changes for this section in commit 2be25cac8402 are
- v = (1 << 30); /* Start of Transaction */
- v |= (1 << 28); /* Write Transaction */
- v |= (1 << 17); /* Turnaround */
- v |= (0x1F << 18);
+ v = BCMA_CORE_PCI_MDIODATA_START;
+ v |= BCMA_CORE_PCI_MDIODATA_WRITE;
+ v |= (BCMA_CORE_PCI_MDIODATA_DEV_ADDR <<
+ BCMA_CORE_PCI_MDIODATA_DEVADDR_SHF);
+ v |= (BCMA_CORE_PCI_MDIODATA_BLK_ADDR <<
+ BCMA_CORE_PCI_MDIODATA_REGADDR_SHF);
+ v |= BCMA_CORE_PCI_MDIODATA_TA;
Because the code has done quite a bit of work on v just above this
section, I agree that this is likely an error, but that error happened
in an earlier commit. Thus 2be25cac8402 did not introduce the error,
merely copied it.
Ugh, this goes back further. I didn't spot that. I'm less confident of
what the correct settings should be now.
Has this change been tested?
Afraid not, I don't have the H/W.
I admit that I looked at this only because I found it hard to believe that the
collective wisdom of the list would have missed the usage of "=" instead of
"|=". At least that test was passed. :)
Larry
Re: [PATCH] bcma: fix incorrect update of BCMA_CORE_PCI_MDIO_DATA
On 8/22/19 8:35 AM, Colin King wrote:
From: Colin Ian King
An earlier commit re-worked the setting of the bitmask and is now
assigning v with some bit flags rather than bitwise or-ing them
into v, consequently the earlier bit-settings of v are being lost.
Fix this by replacing an assignment with the bitwise or instead.
Addresses-Coverity: ("Unused value")
Fixes: 2be25cac8402 ("bcma: add constants for PCI and use them")
Signed-off-by: Colin Ian King
---
drivers/bcma/driver_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bcma/driver_pci.c b/drivers/bcma/driver_pci.c
index f499a469e66d..d219ee947c07 100644
--- a/drivers/bcma/driver_pci.c
+++ b/drivers/bcma/driver_pci.c
@@ -78,7 +78,7 @@ static u16 bcma_pcie_mdio_read(struct bcma_drv_pci *pc, u16
device, u8 address)
v |= (address << BCMA_CORE_PCI_MDIODATA_REGADDR_SHF_OLD);
}
- v = BCMA_CORE_PCI_MDIODATA_START;
+ v |= BCMA_CORE_PCI_MDIODATA_START;
v |= BCMA_CORE_PCI_MDIODATA_READ;
v |= BCMA_CORE_PCI_MDIODATA_TA;
I'm not sure the "Fixes" attribute is correct.
The changes for this section in commit 2be25cac8402 are
- v = (1 << 30); /* Start of Transaction */
- v |= (1 << 28); /* Write Transaction */
- v |= (1 << 17); /* Turnaround */
- v |= (0x1F << 18);
+ v = BCMA_CORE_PCI_MDIODATA_START;
+ v |= BCMA_CORE_PCI_MDIODATA_WRITE;
+ v |= (BCMA_CORE_PCI_MDIODATA_DEV_ADDR <<
+ BCMA_CORE_PCI_MDIODATA_DEVADDR_SHF);
+ v |= (BCMA_CORE_PCI_MDIODATA_BLK_ADDR <<
+ BCMA_CORE_PCI_MDIODATA_REGADDR_SHF);
+ v |= BCMA_CORE_PCI_MDIODATA_TA;
Because the code has done quite a bit of work on v just above this section, I
agree that this is likely an error, but that error happened in an earlier
commit. Thus 2be25cac8402 did not introduce the error, merely copied it.
Has this change been tested?
Larry
Re: [PATCH v2] Fix non-kerneldoc comment in realtek/rtlwifi/usb.c
On 8/8/19 2:10 AM, Kalle Valo wrote:
Larry Finger writes:
On 8/7/19 8:51 PM, Valdis Klētnieks wrote:
Fix spurious warning message when building with W=1:
CC [M] drivers/net/wireless/realtek/rtlwifi/usb.o
drivers/net/wireless/realtek/rtlwifi/usb.c:243: warning: Cannot understand *
on line 243 - I thought it was a doc line
drivers/net/wireless/realtek/rtlwifi/usb.c:760: warning: Cannot understand *
on line 760 - I thought it was a doc line
drivers/net/wireless/realtek/rtlwifi/usb.c:790: warning: Cannot understand *
on line 790 - I thought it was a doc line
Clean up the comment format.
Signed-off-by: Valdis Kletnieks
---
Changes since v1: Larry Finger pointed out the patch wasn't checkpatch-clean.
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c
b/drivers/net/wireless/realtek/rtlwifi/usb.c
index 34d68dbf4b4c..4b59f3b46b28 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -239,10 +239,7 @@ static void _rtl_usb_io_handler_release(struct
ieee80211_hw *hw)
mutex_destroy(>io.bb_mutex);
}
-/**
- *
- * Default aggregation handler. Do nothing and just return the oldest skb.
- */
+/* Default aggregation handler. Do nothing and just return the oldest skb.
*/
static struct sk_buff *_none_usb_tx_aggregate_hdl(struct ieee80211_hw *hw,
struct sk_buff_head *list)
{
@@ -756,11 +753,6 @@ static int rtl_usb_start(struct ieee80211_hw *hw)
return err;
}
-/**
- *
- *
- */
-
/*=== tx =*/
static void rtl_usb_cleanup(struct ieee80211_hw *hw)
{
@@ -786,11 +778,7 @@ static void rtl_usb_cleanup(struct ieee80211_hw *hw)
usb_kill_anchored_urbs(>tx_submitted);
}
-/**
- *
- * We may add some struct into struct rtl_usb later. Do deinit here.
- *
- */
+/* We may add some struct into struct rtl_usb later. Do deinit here. */
static void rtl_usb_deinit(struct ieee80211_hw *hw)
{
rtl_usb_cleanup(hw);
I missed that the subject line should be "rtwifi: Fix ". Otherwise it is OK.
I can fix the subject during commit.
OK. Acked-by: Larry Finger
Thanks,
Larry
Re: [PATCH v2] Fix non-kerneldoc comment in realtek/rtlwifi/usb.c
On 8/7/19 8:51 PM, Valdis Klētnieks wrote:
Fix spurious warning message when building with W=1:
CC [M] drivers/net/wireless/realtek/rtlwifi/usb.o
drivers/net/wireless/realtek/rtlwifi/usb.c:243: warning: Cannot understand *
on line 243 - I thought it was a doc line
drivers/net/wireless/realtek/rtlwifi/usb.c:760: warning: Cannot understand *
on line 760 - I thought it was a doc line
drivers/net/wireless/realtek/rtlwifi/usb.c:790: warning: Cannot understand *
on line 790 - I thought it was a doc line
Clean up the comment format.
Signed-off-by: Valdis Kletnieks
---
Changes since v1: Larry Finger pointed out the patch wasn't checkpatch-clean.
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c
b/drivers/net/wireless/realtek/rtlwifi/usb.c
index 34d68dbf4b4c..4b59f3b46b28 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -239,10 +239,7 @@ static void _rtl_usb_io_handler_release(struct
ieee80211_hw *hw)
mutex_destroy(>io.bb_mutex);
}
-/**
- *
- * Default aggregation handler. Do nothing and just return the oldest skb.
- */
+/* Default aggregation handler. Do nothing and just return the oldest skb.
*/
static struct sk_buff *_none_usb_tx_aggregate_hdl(struct ieee80211_hw *hw,
struct sk_buff_head *list)
{
@@ -756,11 +753,6 @@ static int rtl_usb_start(struct ieee80211_hw *hw)
return err;
}
-/**
- *
- *
- */
-
/*=== tx =*/
static void rtl_usb_cleanup(struct ieee80211_hw *hw)
{
@@ -786,11 +778,7 @@ static void rtl_usb_cleanup(struct ieee80211_hw *hw)
usb_kill_anchored_urbs(>tx_submitted);
}
-/**
- *
- * We may add some struct into struct rtl_usb later. Do deinit here.
- *
- */
+/* We may add some struct into struct rtl_usb later. Do deinit here. */
static void rtl_usb_deinit(struct ieee80211_hw *hw)
{
rtl_usb_cleanup(hw);
I missed that the subject line should be "rtwifi: Fix ". Otherwise it is OK.
Larry
Re: [PATCH] Fix non-kerneldoc comment in realtek/rtlwifi/usb.c
On 8/7/19 5:39 PM, Valdis Klētnieks wrote: When this driver was originally entered, a line with "/*" was flagged by checkpatch.pl. In fact, when I make your change, I get WARNING: networking block comments don't use an empty /* line, use /* Comment... #243: FILE: drivers/net/wireless/realtek/rtlwifi/usb.c:243: +/* + * To avoid a loop of "fixing" compiler/checkpatch warnings, you need to put the first real line of the comment on the line of the "/*". For the first of your patches, that results in diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c index 34d68dbf4b4c..f89ceac25eff 100644 --- a/drivers/net/wireless/realtek/rtlwifi/usb.c +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c @@ -239,10 +239,7 @@ static void _rtl_usb_io_handler_release(struct ieee80211_hw *hw) mutex_destroy(>io.bb_mutex); } -/** - * - * Default aggregation handler. Do nothing and just return the oldest skb. - */ +/* Default aggregation handler. Do nothing and just return the oldest skb. */ static struct sk_buff *_none_usb_tx_aggregate_hdl(struct ieee80211_hw *hw, struct sk_buff_head *list) Because you merely shift the warning to a different tool, NACK. Larry
Re: [PATCH 1/6] staging: rtl8188eu: add spaces around '+' in usb_halinit.c
On 7/26/19 1:04 PM, Michael Straube wrote: Add spaces around '+' to improve readability and follow kernel coding style. Reported by checkpatch. Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/hal/usb_halinit.c | 76 ++--- 1 file changed, 38 insertions(+), 38 deletions(-) If they apply, all six of these are OK. Acked-by: Larry Finger Larry
Re: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
On 7/8/19 1:32 AM, Jian-Hong Pan wrote:
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
b/drivers/net/wireless/realtek/rtw88/pci.c
index cfe05ba7280d..1bfc99ae6b84 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct
rtw_pci *rtwpci,
rx_desc = skb->data;
chip->ops->query_rx_desc(rtwdev, rx_desc, _stat,
_status);
+ /* discard current skb if the new skb cannot be allocated as a
+* new one in rx ring later
+* */
+ new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
+ if (WARN(!new, "rx routine starvation\n")) {
+ new = skb;
+ goto next_rp;
This should probably be a WARN_ONCE() rather than WARN(), otherwise the logs
will be flooded once this condition triggers.
Larry
Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
On 7/4/19 10:44 PM, Daniel Drake wrote: On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen wrote: My point is this seems to be very dongle dependent :( We have to be careful not breaking it for some users while fixing it for others. Do you still have your device? Once we get to the point when you are happy with Chris's two patches here on a code review level, we'll reach out to other driver contributors plus people who previously complained about these types of problems, and see if we can get some wider testing. Larry, do you have these devices, can you help with testing too? I have some of the devices, and I can help with the testing. Larry
Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac
On 6/14/19 2:15 PM, Aaro Koskinen wrote:
Hi,
On Fri, Jun 14, 2019 at 09:24:16AM +0200, Mathieu Malaterre wrote:
On Thu, Jun 13, 2019 at 10:27 AM Christoph Hellwig wrote:
With the strict dma mask checking introduced with the switch to
the generic DMA direct code common wifi chips on 32-bit powerbooks
stopped working. Add a 30-bit ZONE_DMA to the 32-bit pmac builds
to allow them to reliably allocate dma coherent memory.
Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
Reported-by: Aaro Koskinen
Signed-off-by: Christoph Hellwig
---
arch/powerpc/include/asm/page.h | 7 +++
arch/powerpc/mm/mem.c | 3 ++-
arch/powerpc/platforms/powermac/Kconfig | 1 +
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2013b4..0d52f57fca04 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,13 @@ struct vm_area_struct;
#endif /* __ASSEMBLY__ */
#include
+/*
+ * Allow 30-bit DMA for very limited Broadcom wifi chips on many powerbooks.
nit: would it be possible to mention explicit reference to b43-legacy.
Using b43 on my macmini g4 never showed those symptoms (using
5.2.0-rc2+)
According to Wikipedia Mac mini G4 is limited to 1 GB RAM, so that's
why you don't see the issue.
He wouldn't see it with b43. Those cards have 32-bit DMA.
Larry
Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac
On 6/13/19 3:24 AM, Christoph Hellwig wrote:
With the strict dma mask checking introduced with the switch to
the generic DMA direct code common wifi chips on 32-bit powerbooks
stopped working. Add a 30-bit ZONE_DMA to the 32-bit pmac builds
to allow them to reliably allocate dma coherent memory.
Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
Reported-by: Aaro Koskinen
Signed-off-by: Christoph Hellwig
As expected, it works. The patch needs
Cc: Stable # v5.1+
Tested-by: Larry Finger
Acked-by: Larry Finger
Thanks for the help, and the patience with my debugging problems with u64
variables.
Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/12/19 1:55 AM, Christoph Hellwig wrote: Ooops, yes. But I think we could just enable ZONE_DMA on 32-bit powerpc. Crude enablement hack below: diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..1dd71a98b70c 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE config ZONE_DMA bool - default y if PPC_BOOK3E_64 + default y config PGTABLE_LEVELS int With the patch for Kconfig above, and the original patch setting ARCH_ZONE_DMA_BITS to 30, everything works. Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined? Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/11/19 5:46 PM, Aaro Koskinen wrote: Hi, On Tue, Jun 11, 2019 at 05:20:12PM -0500, Larry Finger wrote: It is obvious that the case of a mask smaller than min_mask should be handled by the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All other CONFIG variables containing IOMMU are not selected. When dma_direct_supported() fails, should the system not try for an IOMMU solution? Is the driver asking for the wrong type of memory? It is doing a dma_and_set_mask_coherent() call. I don't think we have IOMMU on G4. On G5 it should work (I remember fixing b43 issue on G5, see 4c374af5fdee, unfortunately all my G5 Macs with b43 are dead and waiting for re-capping). You are right. My configuration has CONFIG_IOMMU_SUPPORT=y, but there is no mention of an IOMMU in the log. Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/11/19 5:46 PM, Benjamin Herrenschmidt wrote: On Tue, 2019-06-11 at 17:20 -0500, Larry Finger wrote: b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask = 0x3fff, min_mask = 0x5000/0x5000, dma bits = 0x1f Ugh ? A mask with holes in it ? That's very wrong... That min_mask is bogus. I agree, but that is not likely serious as most systems will have enough memory that the max_pfn term will be much larger than the initial min_mask, and min_mask will be unchanged by the min function. In addition, min_mask is not used beyond this routine, and then only to decide if direct dma is supported. The following patch generates masks with no holes, but I cannot see that it is needed. diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 2c2772e9702a..e3edd4f29e80 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -384,7 +384,8 @@ int dma_direct_supported(struct device *dev, u64 mask) else min_mask = DMA_BIT_MASK(32); - min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); + min_mask = min_t(u64, min_mask, ((max_pfn - 1) << PAGE_SHIFT) | +DMA_BIT_MASK(PAGE_SHIFT)); /* * This check needs to be against the actual bit mask value, so Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/11/19 1:05 AM, Christoph Hellwig wrote:
On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
What might be confusing in your output is that dev->dma_mask is a pointer,
and we are setting it in dma_set_mask. That is before we only check
if the pointer is set, and later we override it. Of course this doesn't
actually explain the failure. But what is even more strange to me
is that you get a return value from dma_supported() that isn't 0 or 1,
as that function is supposed to return a boolean, and I really can't see
how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
or 1. Does the output change if you use the correct printk specifiers?
i.e. with a debug patch like this:
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..9e5b30b12b10 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
int dma_direct_supported(struct device *dev, u64 mask)
{
u64 min_mask;
+ bool ret;
if (IS_ENABLED(CONFIG_ZONE_DMA))
min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
@@ -391,7 +392,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
* use __phys_to_dma() here so that the SME encryption mask isn't
* part of the check.
*/
- return mask >= __phys_to_dma(dev, min_mask);
+ ret = (mask >= __phys_to_dma(dev, min_mask));
+ if (!ret)
+ dev_info(dev,
+ "%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma
bits = %d\n",
+ __func__, mask, min_mask, __phys_to_dma(dev, min_mask),
ARCH_ZONE_DMA_BITS);
+ return ret;
}
size_t dma_direct_max_mapping_size(struct device *dev)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..6c57ccdee2ae 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
int dma_set_mask(struct device *dev, u64 mask)
{
- if (!dev->dma_mask || !dma_supported(dev, mask))
+ if (!dev->dma_mask) {
+ dev_info(dev, "no DMA mask set!\n");
return -EIO;
+ }
+ if (!dma_supported(dev, mask)) {
+ printk("DMA not supported\n");
+ return -EIO;
+ }
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
After I got the correct formatting, the output with this patch only gives the
following in dmesg:
b43-pci-bridge 0001:11:00.0: dma_direct_supported: failed (mask = 0x3fff,
min_mask = 0x5000/0x5000, dma bits = 0x1f
DMA not supported
b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit
DMA mask
Your first patch did not work as the configuration does not have
CONFIG_ZONE_DMA. As a result, the initial value of min_mask always starts at 32
bits and is taken down to 31 with the maximum pfn minimization. When I forced
the initial value of min_mask to 30 bits, the device worked.
It is obvious that the case of a mask smaller than min_mask should be handled by
the IOMMU. In my system, CONFIG_IOMMU_SUPPORT is selected. All other CONFIG
variables containing IOMMU are not selected. When dma_direct_supported() fails,
should the system not try for an IOMMU solution? Is the driver asking for the
wrong type of memory? It is doing a dma_and_set_mask_coherent() call.
Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote: Please try the attached patch. I'm not really pleased with it and I will continue to determine why the fallback to a 30-bit mask fails, but at least this one works for me. Your patch only makes sense if the device is indeed capable of addressing 31-bits. So either the driver is buggy and asks for a too small mask in which case your patch is ok, or it's not and you're just going to cause all sort of interesting random problems including possible memory corruption. Of course the driver may be buggy, but it asks for the correct mask. This particular device is not capable of handling 32-bit DMA. The driver detects the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32 until 5.1. As Christoph said, it should always be possible to use fewer bits than the maximum. Similar devices that are new enough to use b43 rather than b43legacy work with new kernels; however, they have and use 32-bit DMA. Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/10/19 3:18 AM, Christoph Hellwig wrote:
On Sat, Jun 08, 2019 at 04:52:24PM -0500, Larry Finger wrote:
On 6/7/19 12:29 PM, Christoph Hellwig wrote:
I don't think we should work around this in the driver, we need to fix
it in the core. I'm curious why my previous patch didn't work. Can
you throw in a few printks what failed? I.e. did dma_direct_supported
return false? Did the actual allocation fail?
Routine dma_direct_supported() returns true.
The failure is in routine dma_set_mask() in the following if test:
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
For b43legacy, dev->dma_mask is 0xc2656848.
dma_supported(dev, mask) is 0xc08b, mask is 0x3fff, and
the routine returns -EIO.
For b43, dev->dma_mask is 0xc26568480001,
dma_supported(dev, mask) is 0xc08b, mask is 0x, and
the routine returns 0.
I don't fully understand what values the above map to. Can you send
me your actual debugging patch as well?
I do not understand why the if statement returns true as neither of the values
is zero. After seeing the x86 output shown below, I also do not understand all
the trailing zeros.
My entire patch is attached. That output came from this section:
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask,
dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
On a 32-bit x86 computer with 1GB of RAM, that same output was
For b43legacy, dev->dma_mask is 0x01f4029044.
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0x3fff, and
the routine returns 0. 30-bit DMA works.
For b43, dev->dma_mask is 0x01f4029044,
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0x, and
the routine also returns 0. This card supports 32-bit DMA.
Larry
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2..7a367ce 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ extern void copy_user_page(void *to, void *from, unsigned
long vaddr,
#endif /* __ASSEMBLY__ */
#include
+#if 1 /* XXX: pmac? dynamic discovery? */
+#define ARCH_ZONE_DMA_BITS 30
+#else
#define ARCH_ZONE_DMA_BITS 31
+#endif
#endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 09231ef..761d951 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -20,6 +20,8 @@
*/
static inline bool dma_iommu_alloc_bypass(struct device *dev)
{
+ pr_info("dev->archdata.iommu_bypass %d, !iommu_fixed_is_weak %d\n",
+ dev->archdata.iommu_bypass, !iommu_fixed_is_weak)
return dev->archdata.iommu_bypass && !iommu_fixed_is_weak &&
dma_direct_supported(dev, dev->coherent_dma_mask);
}
@@ -27,6 +29,8 @@ static inline bool dma_iommu_alloc_bypass(struct device *dev)
static inline bool dma_iommu_map_bypass(struct device *dev,
unsigned long attrs)
{
+ pr_info("(attrs & DMA_ATTR_WEAK_ORDERING) %d\n",
+ (attrs & DMA_ATTR_WEAK_ORDERING));
return dev->archdata.iommu_bypass &&
(!iommu_fixed_is_weak || (attrs & DMA_ATTR_WEAK_ORDERING));
}
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index cba2913..2540d3b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -248,7 +248,8 @@ void __init paging_init(void)
(long int)((top_of_ram - total_ram) >> 20));
#ifdef CONFIG_ZONE_DMA
- max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffUL >> PAGE_SHIFT);
+ max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
+ ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
#endif
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
#ifdef CONFIG_HIGHMEM
diff --git a/drivers/net/wireless/broadcom/b43/dma.c
b/drivers/net/wireless/broadcom/b43/dma.c
index 806406a..e0270da 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -1053,6 +1053,7 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64
mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_i
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/7/19 12:29 PM, Christoph Hellwig wrote: I don't think we should work around this in the driver, we need to fix it in the core. I'm curious why my previous patch didn't work. Can you throw in a few printks what failed? I.e. did dma_direct_supported return false? Did the actual allocation fail? Routine dma_direct_supported() returns true. The failure is in routine dma_set_mask() in the following if test: if (!dev->dma_mask || !dma_supported(dev, mask)) return -EIO; For b43legacy, dev->dma_mask is 0xc2656848. dma_supported(dev, mask) is 0xc08b, mask is 0x3fff, and the routine returns -EIO. For b43, dev->dma_mask is 0xc26568480001, dma_supported(dev, mask) is 0xc08b, mask is 0x, and the routine returns 0. Thus far I have not found what sets the low-order bit of dev->dma_mask. Suggestions are welcome. These tests have all been with your patch that sets ARCH_ZONE_DMA_BITS to 30. Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/7/19 12:29 PM, Christoph Hellwig wrote: I don't think we should work around this in the driver, we need to fix it in the core. I'm curious why my previous patch didn't work. Can you throw in a few printks what failed? I.e. did dma_direct_supported return false? Did the actual allocation fail? I agree that that patch should not be sent upstream. I posted it only so that anyone running into the problem would have a work around. I will try to see why your patch failed. Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/5/19 5:50 PM, Aaro Koskinen wrote:
Hi,
When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
not work anymore:
[ 42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14
(2005-04-18 02:36:27)
[ 42.184837] b43legacy-phy0 debug: Chip initialized
[ 42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the
required 30-bit DMA mask
The same happens with the current mainline.
Bisected to:
commit 65a21b71f948406201e4f62e41f06513350ca390
Author: Christoph Hellwig
Date: Wed Feb 13 08:01:26 2019 +0100
powerpc/dma: remove dma_nommu_dma_supported
This function is largely identical to the generic version used
everywhere else. Replace it with the generic version.
Signed-off-by: Christoph Hellwig
Tested-by: Christian Zigotzky
Signed-off-by: Michael Ellerman
Aaro,
Please try the attached patch. I'm not really pleased with it and I will
continue to determine why the fallback to a 30-bit mask fails, but at least this
one works for me.
Larry
>From 25e2f50273e785598b6bd9a8aee28cf825d3fe9f Mon Sep 17 00:00:00 2001
From: Larry Finger
Date: Fri, 7 Jun 2019 12:04:16 -0500
Subject: [PATCH] b43legacy: Fix DMA breakage from commit commit 65a21b71f948
To: kv...@codeaurora.org
Cc: linux-wirel...@vger.kernel.org,
pks...@realtek.com
Following commit 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported"),
this driver errors with a message that "The machine/kernel does not
support the required 30-bit DMA mask." Indeed, the hardware only allows
31-bit masks. This solution is to change the fallback mask from 30-
to 31-bits for 32-bit PPC systems.
Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
Reported-by: Aaro Koskinen
Cc: Christoph Hellwig
Cc: Aaro Koskinen
Signed-off-by: Larry Finger
---
drivers/net/wireless/broadcom/b43legacy/dma.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 1cc25f44dd9a..75613f516e50 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -27,6 +27,15 @@
#include
#include
+/* Special handling for PPC32 - The maximum DMA mask is 31 bits, and
+ * the fallback to 30 bits fails. Set the fallback at 31.
+ */
+#ifdef CONFIG_PPC32
+#define FB_DMA 31
+#else
+#define FB_DMA 30
+#endif
+
/* 32bit DMA ops. */
static
struct b43legacy_dmadesc32 *op32_idx2desc(struct b43legacy_dmaring *ring,
@@ -418,7 +427,7 @@ static bool b43legacy_dma_mapping_error(struct b43legacy_dmaring *ring,
switch (ring->type) {
case B43legacy_DMA_30BIT:
- if ((u64)addr + buffersize > (1ULL << 30))
+ if ((u64)addr + buffersize > (1ULL << FB_DMA))
goto address_error;
break;
case B43legacy_DMA_32BIT:
@@ -617,12 +626,12 @@ static u64 supported_dma_mask(struct b43legacy_wldev *dev)
if (tmp & B43legacy_DMA32_TXADDREXT_MASK)
return DMA_BIT_MASK(32);
- return DMA_BIT_MASK(30);
+ return DMA_BIT_MASK(FB_DMA);
}
static enum b43legacy_dmatype dma_mask_to_engine_type(u64 dmamask)
{
- if (dmamask == DMA_BIT_MASK(30))
+ if (dmamask == DMA_BIT_MASK(FB_DMA))
return B43legacy_DMA_30BIT;
if (dmamask == DMA_BIT_MASK(32))
return B43legacy_DMA_32BIT;
@@ -802,7 +811,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
continue;
}
if (mask == DMA_BIT_MASK(32)) {
- mask = DMA_BIT_MASK(30);
+ mask = DMA_BIT_MASK(FB_DMA);
fallback = true;
continue;
}
--
2.21.0
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/6/19 6:43 AM, Christoph Hellwig wrote: On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote: Wow... that's an odd amount. One thing we could possibly do is add code to limit the amount of RAM when we detect that device Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based on detecting the presence of that device in the device-tree. swiotlb doesn't really help you, as these days swiotlb on matters for the dma_map* case. What would help is a ZONE_DMA that covers these devices. No need to do the 24-bit x86 does, but 30-bit would do it. WIP patch for testing below: diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index b8286a2013b4..7a367ce87c41 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -319,6 +319,10 @@ struct vm_area_struct; #endif /* __ASSEMBLY__ */ #include +#if 1 /* XXX: pmac? dynamic discovery? */ +#define ARCH_ZONE_DMA_BITS 30 +#else #define ARCH_ZONE_DMA_BITS 31 +#endif #endif /* _ASM_POWERPC_PAGE_H */ diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index cba29131bccc..2540d3b2588c 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -248,7 +248,8 @@ void __init paging_init(void) (long int)((top_of_ram - total_ram) >> 20)); #ifdef CONFIG_ZONE_DMA - max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffUL >> PAGE_SHIFT); + max_zone_pfns[ZONE_DMA] = min(max_low_pfn, + ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT); #endif max_zone_pfns[ZONE_NORMAL] = max_low_pfn; #ifdef CONFIG_HIGHMEM This trial patch failed. Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/6/19 6:43 AM, Christoph Hellwig wrote: On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote: Wow... that's an odd amount. One thing we could possibly do is add code to limit the amount of RAM when we detect that device Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based on detecting the presence of that device in the device-tree. swiotlb doesn't really help you, as these days swiotlb on matters for the dma_map* case. What would help is a ZONE_DMA that covers these devices. No need to do the 24-bit x86 does, but 30-bit would do it. WIP patch for testing below: diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index b8286a2013b4..7a367ce87c41 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -319,6 +319,10 @@ struct vm_area_struct; #endif /* __ASSEMBLY__ */ #include +#if 1 /* XXX: pmac? dynamic discovery? */ +#define ARCH_ZONE_DMA_BITS 30 +#else #define ARCH_ZONE_DMA_BITS 31 +#endif #endif /* _ASM_POWERPC_PAGE_H */ diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index cba29131bccc..2540d3b2588c 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -248,7 +248,8 @@ void __init paging_init(void) (long int)((top_of_ram - total_ram) >> 20)); #ifdef CONFIG_ZONE_DMA - max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffUL >> PAGE_SHIFT); + max_zone_pfns[ZONE_DMA] = min(max_low_pfn, + ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT); #endif max_zone_pfns[ZONE_NORMAL] = max_low_pfn; #ifdef CONFIG_HIGHMEM I am generating a test kernel with this patch. FYI, the "free" command on my machine shows 1.5+ G of memory. That likely means I have 2G installed. I have tested a patched kernel in which b43legacy falls back to a 31-bit DMA mask when the 32-bit one failed. That worked, but would likely kill the x86 version. Let me know if think a fix in the driver rather than the kernel would be better. I still need to understand why the same setup works in b43 and fails in b43legacy. :( Larry
Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
On 6/5/19 5:50 PM, Aaro Koskinen wrote: Hi, When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does not work anymore: [ 42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27) [ 42.184837] b43legacy-phy0 debug: Chip initialized [ 42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask The same happens with the current mainline. Bisected to: commit 65a21b71f948406201e4f62e41f06513350ca390 Author: Christoph Hellwig Date: Wed Feb 13 08:01:26 2019 +0100 powerpc/dma: remove dma_nommu_dma_supported This function is largely identical to the generic version used everywhere else. Replace it with the generic version. Signed-off-by: Christoph Hellwig Tested-by: Christian Zigotzky Signed-off-by: Michael Ellerman Aaro, First of all, you have my sympathy for the laborious bisection on a PowerBook G4. I have done several myself. Thank you. I confirm your results. The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will fail. Why the 30-bit fallback fails in b43legacy fails while it works in b43 is a mystery. Although dma_nommu_dma_supported() may be "largely identical" to dma_direct_supported(), they obviously differ. Routine dma_nommu_dma_supported() returns 1 for 32-bit systems, but I do not know what dma_direct_supported() returns. I am trying to find a patch. Larry
Re: [PATCH] rtlwifi: remove redundant assignment to variable k
On 5/31/19 9:14 AM, Colin King wrote:
From: Colin Ian King
The assignment of 0 to variable k is never read once we break out of
the loop, so the assignment is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c
b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..83e5318ca04f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
rtlpriv->cfg->
maps[EFUSE_CTRL] + 3);
k++;
- if (k == 1000) {
- k = 0;
+ if (k == 1000)
break;
- }
}
data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
return data;
Colin,
Your patch is not wrong, but it fails to address a basic deficiency of this code
snippet - when an error is detected, there is no attempt to either fix the
problem or report it upstream. As the data returned will be garbage if this
condition happens, we might as well replace the "break" with "return 0xFF", as
well as deleting the "k = 0" line. Most of the callers of efuse_read_1byte()
ignore the returned result when bits 0 and 4 are set, thus returning all 8 bits
is not a bad fixup.
My suspicion is that this test is in the code merely to prevent an potential
unterminated "while" loop, and that this condition is extremely unlikely to happen.
Larry
Re: [PATCH] rtlwifi: remove redundant assignment to variable badworden
On 5/30/19 1:40 PM, Colin King wrote:
From: Colin Ian King
The variable badworden is assigned with a value that is never read and
it is re-assigned a new value immediately afterwards. The assignment is
redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
Acked-by: Larry Finger
Thanks,
Larry
drivers/net/wireless/realtek/rtlwifi/efuse.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c
b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..37ab582a8afb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -986,7 +986,6 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
} else if (write_state == PG_STATE_DATA) {
RTPRINT(rtlpriv, FEEPROM, EFUSE_PG,
"efuse PG_STATE_DATA\n");
- badworden = 0x0f;
badworden =
enable_efuse_data_write(hw, efuse_addr + 1,
target_pkt.word_en,
Re: [RFC PATCH v3] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
On 5/29/19 12:03 AM, Chris Chiu wrote: 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 improve. The driver at GitHub was written by Realtek. I only published it in a prominent location, and fix it for kernel API changes. I would say "the Realtek driver at https://...;, and every mention of "Larry's driver" should say "Realtek's driver". That attribution is more correct. 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. With this commit, the tx rate of each data and qos data packet will be 39Mbps (MCS4) with the 0xF0 as the tx rate mask. The 20th bit to 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 always 0xFFF 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. Signed-off-by: Chris Chiu I have not tested this patch, but I plan to soon. Larry
Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
On 5/29/19 5:30 AM, Jia-Ju Bai wrote: On 2019/5/28 21:00, Larry Finger wrote: On 5/28/19 6:55 AM, Kalle Valo wrote: Jia-Ju Bai wrote: *BUG 1: In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails, rtl_deinit_core() in the error handling code is executed. rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been initialized. Thus a null-pointer dereference occurs. The reason is that rtlpriv->scan_list.list is initialized by INIT_LIST_HEAD() in rtl_init_core(), which has not been called. To fix this bug, rtl_deinit_core() should not be called when rtlpriv->cfg->ops->init_sw_vars() fails. *BUG 2: In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in this function fails, and rtlpriv->scan_list.list has not been initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error handling code of rtl_pci_probe() is executed. Finally, a null-pointer dereference occurs due to the same reason of the above bug. To fix this bug, the initialization of lists in rtl_init_core() are performed before the call to rtl_regd_init(). These bugs are found by a runtime fuzzing tool named FIZZER written by us. Signed-off-by: Jia-Ju Bai Ping & Larry, is this ok to take? Kalle, Not at the moment. In reviewing the code, I was unable to see how this situation could develop, and his backtrace did not mention any rtlwifi code. For that reason, I asked him to add printk stat4ements to show the last part of rtl_pci that executed correctly. In https://marc.info/?l=linux-wireless=155788322631134=2, he promised to do that, but I have not seen the result. Hi Larry, This patch is not related to the message you mentioned. That message is about an occasional crash that I reported. That crash occurred when request_irq() in rtl_pci_intr_mode_legacy() in rtl_pci_intr_mode_decide() fails. I have added printk statements and try to reproduce and debug that crash, but that crash does not always occur, and I still do not know the root cause of that crash. The null-pointer dereferences fixed by this patch are different from that crash, and they always occur when the related functions fail. So please review these null-pointer dereferences, thanks :) Best wishes, Jia-Ju Bai Sorry if I got confused. Kalle has dropped this patch, thus you will need to submit a new version. Larry
Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
On 5/28/19 6:55 AM, Kalle Valo wrote: Jia-Ju Bai wrote: *BUG 1: In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails, rtl_deinit_core() in the error handling code is executed. rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been initialized. Thus a null-pointer dereference occurs. The reason is that rtlpriv->scan_list.list is initialized by INIT_LIST_HEAD() in rtl_init_core(), which has not been called. To fix this bug, rtl_deinit_core() should not be called when rtlpriv->cfg->ops->init_sw_vars() fails. *BUG 2: In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in this function fails, and rtlpriv->scan_list.list has not been initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error handling code of rtl_pci_probe() is executed. Finally, a null-pointer dereference occurs due to the same reason of the above bug. To fix this bug, the initialization of lists in rtl_init_core() are performed before the call to rtl_regd_init(). These bugs are found by a runtime fuzzing tool named FIZZER written by us. Signed-off-by: Jia-Ju Bai Ping & Larry, is this ok to take? Kalle, Not at the moment. In reviewing the code, I was unable to see how this situation could develop, and his backtrace did not mention any rtlwifi code. For that reason, I asked him to add printk stat4ements to show the last part of rtl_pci that executed correctly. In https://marc.info/?l=linux-wireless=155788322631134=2, he promised to do that, but I have not seen the result. Larry
Re: [PATCH] staging: Add rtl8821ce PCIe WiFi driver
On 5/15/19 8:06 AM, Kai-Heng Feng wrote: at 20:33, Greg KH wrote: On Wed, May 15, 2019 at 07:54:58PM +0800, Kai-Heng Feng wrote: at 19:40, Greg KH wrote: On Wed, May 15, 2019 at 07:24:01PM +0800, Kai-Heng Feng wrote: The rtl8821ce can be found on many HP and Lenovo laptops. Users have been using out-of-tree module for a while, The new Realtek WiFi driver, rtw88, will support rtl8821ce in 2020 or later. Where is that driver, and why is it going to take so long to get merged? rtw88 is in 5.2 now, but it doesn’t support 8821ce yet. They plan to add the support in 2020. Who is "they" and what is needed to support this device and why wait a full year? “They” refers to Realtek. It’s their plan so I can’t really answer that on behalf of Realtek. 296 files changed, 206166 insertions(+) Ugh, why do we keep having to add the whole mess for every single one of these devices? Because Realtek devices are unfortunately ubiquitous so the support is better come from kernel. That's not the issue here. The issue is that we keep adding the same huge driver files to the kernel tree, over and over, with no real change at all. We have seen almost all of these files in other realtek drivers, right? Yes. They use one single driver to support different SoCs, different architectures and even different OSes. That’s why it’s a mess. Why not use the ones we already have? It’s virtually impossible because Realtek’s mega wifi driver uses tons of #ifdefs, only one chip can be selected to be supported at compile time. But better yet, why not add proper support for this hardware and not use a staging driver? Realtek plans to add the support in 2020, if everything goes well. Meanwhile, many users of HP and Lenovo laptops are using out-of-tree driver, some of them are stuck to older kernels because they don’t know how to fix the driver. So I strongly think having this in kernel is beneficial to many users, even it’s only for a year. Why not solve the older kernel problem the way I do with drivers for many Realtek devices by creating a GitHub project with the kernel API changes properly handled by ifdef statements? See the lwfinger projects. That solves the problem of users without the skills needed to adjust to kernel changes without burdening the entire Linux kernel with these bloated drivers. There are no reasons that a wifi driver should require 200K lines of code! Larry
Re: [PATCH] staging: rtl8*: use help instead of ---help--- in Kconfig
On 4/20/19 6:37 AM, MosesChristopher wrote: From: Moses Christopher - Resolve the following warning from the Kconfig, "WARNING: prefer 'help' over '---help---' for new help texts" Signed-off-by: Moses Christopher I have never seen this warning, but your Kconfig may be newer than mine. In any case, the changes are harmless. Acked-by: Larry Finger --- drivers/staging/rtl8188eu/Kconfig | 4 ++-- drivers/staging/rtl8192e/Kconfig | 8 drivers/staging/rtl8712/Kconfig | 4 ++-- drivers/staging/rtl8723bs/Kconfig | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/staging/rtl8188eu/Kconfig b/drivers/staging/rtl8188eu/Kconfig index ff7832798a77..3f8c655856bf 100644 --- a/drivers/staging/rtl8188eu/Kconfig +++ b/drivers/staging/rtl8188eu/Kconfig @@ -7,7 +7,7 @@ config R8188EU select LIB80211 select LIB80211_CRYPT_WEP select LIB80211_CRYPT_CCMP - ---help--- + help This option adds the Realtek RTL8188EU USB device such as TP-Link TL-WN725N. If built as a module, it will be called r8188eu. @@ -16,7 +16,7 @@ if R8188EU config 88EU_AP_MODE bool "Realtek RTL8188EU AP mode" default y - ---help--- + help This option enables Access Point mode. Unless you know that your system will never be used as an AP, or the target system has limited memory, "Y" should be selected. diff --git a/drivers/staging/rtl8192e/Kconfig b/drivers/staging/rtl8192e/Kconfig index 4602a47cdb4a..aa571c12678f 100644 --- a/drivers/staging/rtl8192e/Kconfig +++ b/drivers/staging/rtl8192e/Kconfig @@ -3,7 +3,7 @@ config RTLLIB depends on WLAN && m default n select LIB80211 - ---help--- + help If you have a wireless card that uses rtllib, say Y. Currently the only card is the rtl8192e. @@ -16,7 +16,7 @@ config RTLLIB_CRYPTO_CCMP depends on RTLLIB select CRYPTO_AES default y - ---help--- + help CCMP crypto driver for rtllib. If you enabled RTLLIB, you want this. @@ -27,7 +27,7 @@ config RTLLIB_CRYPTO_TKIP select CRYPTO_ARC4 select CRYPTO_MICHAEL_MIC default y - ---help--- + help TKIP crypto driver for rtllib. If you enabled RTLLIB, you want this. @@ -37,7 +37,7 @@ config RTLLIB_CRYPTO_WEP select CRYPTO_ARC4 depends on RTLLIB default y - ---help--- + help TKIP crypto driver for rtllib. If you enabled RTLLIB, you want this. diff --git a/drivers/staging/rtl8712/Kconfig b/drivers/staging/rtl8712/Kconfig index f160eee52f09..b377d90742db 100644 --- a/drivers/staging/rtl8712/Kconfig +++ b/drivers/staging/rtl8712/Kconfig @@ -4,14 +4,14 @@ config R8712U select WIRELESS_EXT select WEXT_PRIV select FW_LOADER - ---help--- + help This option adds the Realtek RTL8712 USB device such as the D-Link DWA-130. If built as a module, it will be called r8712u. config R8712_TX_AGGR bool "Realtek RTL8712U Transmit Aggregation code" depends on R8712U && BROKEN - ---help--- + help This option provides transmit aggregation for the Realtek RTL8712 USB device. diff --git a/drivers/staging/rtl8723bs/Kconfig b/drivers/staging/rtl8723bs/Kconfig index deae0427ba6c..41ad6ed24860 100644 --- a/drivers/staging/rtl8723bs/Kconfig +++ b/drivers/staging/rtl8723bs/Kconfig @@ -4,7 +4,7 @@ config RTL8723BS depends on m select WIRELESS_EXT select WEXT_PRIV - ---help--- + help This option enables support for RTL8723BS SDIO drivers, such as the wifi found on the 1st gen Intel Compute Stick, the CHIP and many other Intel Atom and ARM based devices.
Re: [PATCH] powerpc/rtas: fix early boot failure.
On 3/25/19 3:43 AM, Christophe Leroy wrote:
Commit 0df977eafc79 ("powerpc/6xx: Don't use SPRN_SPRG2 for storing
stack pointer while in RTAS") changes the code to use a field in
thread struct to store the stack pointer while in RTAS instead of
using SPRN_SPRG2. It therefore converts all places which were
manipulating SPRN_SPRG2 to use that field. During early startup,
the zeroing of SPRN_SPRG2 has been replaced by a zeroing of that
field in thread struct. But at least in start_here, that's done
wrongly because it used the physical address of the fields while
MMU is on at that time.
So the virtual address of the field should be used instead, but in
the meantime, thread struct has already been zeroised and initialised
so we can just drop this initialisation.
Reported-by: Larry Finger
Fixes: 0df977eafc79 ("powerpc/6xx: Don't use SPRN_SPRG2 for storing stack pointer
while in RTAS")
Signed-off-by: Christophe Leroy
Tested-by: Larry Finger
My PPC box now boots OK.
Larry
Re: Regression in 5.1.0-rc2: PowerBook G4 Aluminum fails to boot - bisected to commit 0df977eafc792
On 3/25/19 3:46 AM, Christophe Leroy wrote: Le 25/03/2019 à 01:49, Larry Finger a écrit : A build of kernel 5.1.0-rc2 resulted in a failure to boot on my PowerBook G4 Aluminum. The bootstrap loads the initial kernel and issues the appropriate start, but the machine hangs at that point. The problem does not depend on the choice of PPC32 processor type. This machine has a 7447A according to /proc/cpuinfo. The problem was bisected to the following: commit 0df977eafc792a5365a7f81d8d5920132e03afad Author: Christophe Leroy Date: Thu Feb 21 10:37:54 2019 + powerpc/6xx: Don't use SPRN_SPRG2 for storing stack pointer while in RTAS When calling RTAS, the stack pointer is stored in SPRN_SPRG2 in order to be able to restore it in case of machine check in RTAS. As machine check is not a perfomance critical path, this patch frees SPRN_SPRG2 by using a field in thread struct instead. Signed-off-by: Christophe Leroy Signed-off-by: Michael Ellerman I reverted this patch and found that the system began execution, and then failed, likely due to the reassignment of SPRN_SPRG2. I had found this problem with 5.1.0-rc1, but -rc2 was out by the time I finished the bisection. Unfortunately, none of the changes in -rc2 fixed the problem. I think I identified the issue, see https://patchwork.ozlabs.org/patch/1063962/ It is now booting properly under QEMU with your .config Could you please test it on your target ? Thanks. That patch fixes the issue. Larry
Re: Regression in 5.1.0-rc2: PowerBook G4 Aluminum fails to boot - bisected to commit 0df977eafc792
On 3/25/19 1:53 AM, Christophe Leroy wrote: Le 25/03/2019 à 01:49, Larry Finger a écrit : A build of kernel 5.1.0-rc2 resulted in a failure to boot on my PowerBook G4 Aluminum. The bootstrap loads the initial kernel and issues the appropriate start, but the machine hangs at that point. Can you please be more explicit ? What do you mean by "issues the appropriate start" ? What is "that point" ? Any messages on the console ? Sorry to be unclear. The bootstrap code prints a line saying "Booting Linux via __start() @ 0x00c0" and then hangs. Your idea of confusion between physical and virtual address sounds right. I am building a kernel with that patch applied now. As it seems to be going quickly, I should have the answer fairly quickly. Thanks, Larry
Re: [PATCH] b43: shut up clang -Wuninitialized variable warning
On 3/22/19 9:37 AM, Arnd Bergmann wrote:
Clang warns about what is clearly a case of passing an uninitalized
variable into a static function:
drivers/net/wireless/broadcom/b43/phy_lp.c:1852:23: error: variable 'gains' is
uninitialized when used here
[-Werror,-Wuninitialized]
lpphy_papd_cal(dev, gains, 0, 1, 30);
^
drivers/net/wireless/broadcom/b43/phy_lp.c:1838:2: note: variable 'gains' is
declared here
struct lpphy_tx_gains gains, oldgains;
^
1 error generated.
However, this function is empty, and its arguments are never evaluated,
so gcc in contrast does not warn here. Both compilers behave in a
reasonable way as far as I can tell, so we should change the code
to avoid the warning everywhere.
We could just eliminate the lpphy_papd_cal() function entirely,
given that it has had the TODO comment in it for 10 years now
and is rather unlikely to ever get done. I'm doing a simpler
change here, and just pass the 'oldgains' variable in that has
been initialized, based on the guess that this is what was
originally meant.
Fixes: 2c0d6100da3e ("b43: LP-PHY: Begin implementing calibration & software RFKILL
support")
Signed-off-by: Arnd Bergmann
---
drivers/net/wireless/broadcom/b43/phy_lp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_lp.c
b/drivers/net/wireless/broadcom/b43/phy_lp.c
index 46408a560814..aedee026c5e2 100644
--- a/drivers/net/wireless/broadcom/b43/phy_lp.c
+++ b/drivers/net/wireless/broadcom/b43/phy_lp.c
@@ -1835,7 +1835,7 @@ static void lpphy_papd_cal(struct b43_wldev *dev, struct
lpphy_tx_gains gains,
static void lpphy_papd_cal_txpwr(struct b43_wldev *dev)
{
struct b43_phy_lp *lpphy = dev->phy.lp;
- struct lpphy_tx_gains gains, oldgains;
+ struct lpphy_tx_gains oldgains;
int old_txpctl, old_afe_ovr, old_rf, old_bbmult;
lpphy_read_tx_pctl_mode_from_hardware(dev);
@@ -1849,9 +1849,9 @@ static void lpphy_papd_cal_txpwr(struct b43_wldev *dev)
lpphy_set_tx_power_control(dev, B43_LPPHY_TXPCTL_OFF);
if (dev->dev->chip_id == 0x4325 && dev->dev->chip_rev == 0)
- lpphy_papd_cal(dev, gains, 0, 1, 30);
+ lpphy_papd_cal(dev, oldgains, 0, 1, 30);
else
- lpphy_papd_cal(dev, gains, 0, 1, 65);
+ lpphy_papd_cal(dev, oldgains, 0, 1, 65);
if (old_afe_ovr)
lpphy_set_tx_gains(dev, oldgains);
Acked-by: Larry Finger
Thanks. I will submit a patch that removes lpphy_papd_cal(). You are correct
that it is unlikely ever to be implemented.
Larry
Re: Realtek r8822be kernel module does not negotiate 802.11ac connection
On 3/2/19 12:09 PM, David R. Bergstein wrote: Larry, I tried using iw but it gives the same reading for bit rate. In regard to the firmware, it was not installed via "make install" so I did it manually. David, There was a typo in the Makefile. 'make install' now installs the firmware correctly. Larry
Re: Realtek r8822be kernel module does not negotiate 802.11ac connection
On 3/1/19 9:52 PM, David R. Bergstein wrote: Larry, Sorry about all these extra replies. Shortly after I sent my last message my access point started recognizing the connection as 802.11ac with PHY Rate / Modulation Rate of 866.6 Mbps. What is somewhat misleading is the information reported by iwconfig (see bit rate below). $ iwconfig wlo1 wlo1 IEEE 802.11 ESSID:"XX-5G" Mode:Managed Frequency:5.22 GHz Access Point: xx:xx:xx:xx:xx:xx Bit Rate=6.5 Mb/s Tx-Power=23 dBm Retry short limit:7 RTS thr:off Fragment thr:off Power Management:on Link Quality=70/70 Signal level=-40 dBm Rx invalid nwid:0 Rx invalid crypt:0 Rx invalid frag:0 Tx excessive retries:0 Invalid misc:11 Missed beacon:0 The utility iwconfig is obsolete and not exactly up to date with the latest in wireless. I am not an expert on iw, but I think you want 'iw dev wlo1 link'. I am wondering about the missing firmware stage. Had you not done the 'sudo make install' step, or did that step fail to install the firmware? Larry
Re: Realtek r8822be kernel module does not negotiate 802.11ac connection
On 3/1/19 4:26 PM, David R. Bergstein wrote: Larry, Thanks for the response and detailed instructions, which allowed me to build and install the rtw88 kernel module. I cannot however seem to get my system to actually use the module. Just to recap this is an HP Omen laptop with secure boot disabled. Upon boot-up both the new rtw88 and old r8822be modules are loaded. If I unload the r8822be module the wifi network connection gets terminated, even if I unload/reload the rtw88 module. Is there something else I should be doing prior to invoking rtw88, e.g., blacklisting the old module? Yes, r8822be must be blacklisted. Use the lsmod command to see what modules are actually loaded. You load/unload rtw88 using the rtwpci module. Larry
Re: Realtek r8822be kernel module does not negotiate 802.11ac connection
On 2/28/19 8:32 PM, David R. Bergstein wrote: Tony, Thanks for your response. Can you advise as to the availability of the new rtw88 driver? As it appears to be under development, I could not locate a copy of the code for local compilation. David, Use the command 'git clone http://github.com/lwfinger/rtlwifi_new.git -b rtw88' to get a copy of the new driver for RTL8822BE and RTL8822CE in a stand-alone form. All that is needed is 'cd rtlwifi_new && make && sudo make install'. This version will build on kernels v4.4 and newer. As I have time, I will be backporting to kernels at least as old as 4.0. I would appreciate your comments and bug reports. Larry
Re: [PATCH] b43: no need to check return value of debugfs_create functions
On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Kalle Valo Cc: linux-wirel...@vger.kernel.org Cc: b43-...@lists.infradead.org Signed-off-by: Greg Kroah-Hartman Acked-by: Larry Finger Thanks, Larry
Re: [PATCH] b43legacy: no need to check return value of debugfs_create functions
On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Larry Finger Cc: Kalle Valo Cc: linux-wirel...@vger.kernel.org Cc: b43-...@lists.infradead.org Signed-off-by: Greg Kroah-Hartman Acked-by: Larry Finger Thanks, Larry
Re: [PATCH] realtek: no need to check return value of debugfs_create functions
On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Ping-Ke Shih Cc: Kalle Valo Cc: linux-wirel...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman Greg, Please change the subject to read "rtlwifi: ...". Otherwise the patch is correct. Acked-by: Larry Finger Thanks, Larry
Re: [PATCH -next] staging: rtl8712: drop pointless static qualifier in r8712_efuse_pg_packet_write()
On 1/21/19 1:53 AM, YueHaibing wrote: There is no need to have the 'intrepeat_times' variable static since new value always be assigned before use it. Signed-off-by: YueHaibing --- drivers/staging/rtl8712/rtl8712_efuse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c b/drivers/staging/rtl8712/rtl8712_efuse.c index 8bc45ff..39eb743 100644 --- a/drivers/staging/rtl8712/rtl8712_efuse.c +++ b/drivers/staging/rtl8712/rtl8712_efuse.c @@ -358,7 +358,7 @@ u8 r8712_efuse_pg_packet_write(struct _adapter *padapter, const u8 offset, u8 pg_header = 0; u16 efuse_addr = 0, curr_size = 0; u8 efuse_data, target_word_cnts = 0; - static int repeat_times; + int repeat_times; int sub_repeat; u8 bResult = true; Clearly, the value of this variable is not intended to be carried over between calls to this routine. Your fix is correct. Acked-by: Larry Finger Thanks, Larry
Re: [PATCH] rtlwifi: rtl818x: fix indentation issue
On 1/17/19 1:29 PM, Joe Perches wrote:
On Thu, 2019-01-17 at 15:28 +, Colin King wrote:
From: Colin Ian King
There is a statement that is indented too deeply. Fix this.
Thanks.
diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
[]
@@ -803,7 +803,7 @@ static void rtl8180_config_cardbus(struct ieee80211_hw *dev)
rtl818x_iowrite16(priv, FEMR_SE, 0x);
} else {
reg16 = rtl818x_ioread16(priv, >map->FEMR);
- reg16 |= (1 << 15) | (1 << 14) | (1 << 4);
+ reg16 |= (1 << 15) | (1 << 14) | (1 << 4);
rtl818x_iowrite16(priv, >map->FEMR, reg16);
}
trivia:
It sure looks as if there could be some rather useful
conversions of magic bits to constants one day.
How much work is warranted for this driver for a device that is not likely in
use anywhere in the wild? In addition, I'm not sure anyone knows what those bits
actually do, I certainly do not have a product sheet for that one.
Larry
Re: [PATCH v4 0/4] rtlwifi: Fix issues with rtl8723ae
On 1/8/19 4:49 PM, Bernd Edlinger wrote: Currently the rtl8723ae driver is broken (since v4.7). Connection to AP is lost very often, especially when the signal level is not very good. The main issue is the power save mode is basically not working, and seems to trigger a firmware bug. So I had to take out the FW LPS mode handling. While debugging the driver I found a couple related issues, for instance that the signal level in dm.undec_sm_pwdb is no longer accurate (may be even much too high) when no more packets are received, and it increases the likelihood to receive something if the input gain is set to maximum. The patch was tested with the rtl8723ae PCI card in my laptop against a FRITZ!Box 7590 AP -- the WiFi connection works now very reliable for me. ChangeLog: v2: Adjusts the defaults of swlps and fwlps module parameters to match the firmware capabilities instead of removing the whole code, so it can be easily re-activated once a firmware update is available. v3: Make the title of each patch fit in one line. v4: Try to fix the line endings the message body. Which is an exchange server issue. The patch does not change at all. Bernd Edlinger (4): rtl8723ae: Take the FW LPS mode handling out rtl8723ae: Dont use old data for input gain control rtl8723ae: Re-introduce the adaptive rate control rtlwifi: Don't clear num_rx_inperiod too early drivers/net/wireless/realtek/rtlwifi/base.c| 4 +- drivers/net/wireless/realtek/rtlwifi/core.c| 2 + .../net/wireless/realtek/rtlwifi/rtl8723ae/dm.c| 95 +- .../net/wireless/realtek/rtlwifi/rtl8723ae/sw.c| 8 +- 4 files changed, 101 insertions(+), 8 deletions(-) This version applied OK. Larry
Re: [PATCH] staging: rtl8188eu: Add device code for D-Link DWA-121 rev B1
On 1/7/19 11:28 AM, Michael Straube wrote: This device was added to the stand-alone driver on github. Add it to the staging driver as well. Link: https://github.com/lwfinger/rtl8188eu/commit/a0619a07cd1e Signed-off-by: Michael Straube Acked-by: Larry Finger Thanks, Larry
Re: [PATCH v2 0/4] rtlwifi: Fix issues with rtl8723ae
On 1/5/19 12:38 PM, Bernd Edlinger wrote: Currently the rtl8723ae driver is broken (since v4.7). Connection to AP is lost very often, especially when the signal level is not very good. The main issue is the power save mode is basically not working, and seems to trigger a firmware bug. So I had to take out the FW LPS mode handling. While debugging the driver I found a couple related issues, for instance that the signal level in dm.undec_sm_pwdb is no longer accurate (may be even much too high) when no more packets are received, and it increases the likelihood to receive something if the input gain is set to maximum. The patch was tested with the rtl8723ae PCI card in my laptop against a FRITZ!Box 7590 AP -- the WiFi connection works now very reliable for me. V2 of the patch adjusts the defaults of swlps and fwlps module parameters to match the firmware capabilities instead of removing the whole code, so it can be easily re-activated once a firmware update is available. Bernd Edlinger (4): rtlwifi: rtl8723ae: Take the FW LPS mode handling out rtlwifi: rtl8723ae: Don't use dm.undec_sm_pwdb for input gain control when no beacon was received in the connected state rtlwifi: rtl8723ae: Re-introduce rtl8723e_dm_refresh_rate_adaptive_mask rtlwifi: Move the clearing of rtlpriv->link_info.num_rx_inperiod in rtl_watchdog_wq_callback a few lines down The subject on all 4 of these need to be modified. Only the first line will be shown as the "subject", which will not make much sense. In addition, you need to include the changes in the cover letter, and in the actual patch after the --- line. In that section, you mention what changes were made in each revision, then follow with another --- line. That info is there for the maintainers - it will be stripped before the patch is merged. Larry
Re: [PATCH 1/4] rtlwifi: rtl8723ae: Take the FW LPS mode handling out
On 1/5/19 10:30 AM, Bernd Edlinger wrote:
On 1/5/19 5:13 PM, Larry Finger wrote:
but this works:
modprobe rtl8723ae debug_mask=0x debug_level=5 swlps=1 fwlps=0
Yes, I think that is a better thing to do now. If and when Realtek finds a
firmware bug, and when the new firmware is readily available, then there will
not be a lot of code to reinstall.
Okay, my assumption of how the firmware bug "works" is this:
Once the firmware enters power save mode, it will deliver exactly one (or maybe
two)
Rx interrupts. If one of those triggers the driver to leave the power save mode
again,
the firmware continues to work.
If those are only beacons, they won't leave the power save mode. Then the
firmware
will usually not recover.
Since prior to commit 873ffe154ae0 ("rtlwifi: Fix logic error in enter/exit
power-save mode")
the power save mode was only activated when there _is_ busy traffic, the next
packet did usually wake the firmware, rarely it did freeze however.
Other things like changing the cck_packet_detection_threshold or
refresh_rate_adaptive_mask
can also kick the firmware back to life.
Hope this helps to track down the root cause of this bug.
As I know nothing of the firmware, that is outside my expertise, but that
observation should be of great help to PK Shih, and his colleagues at Realtek.
Larry
Re: [PATCH 1/4] rtlwifi: rtl8723ae: Take the FW LPS mode handling out
On 1/5/19 5:31 AM, Bernd Edlinger wrote:
On 1/5/19 3:44 AM, Larry Finger wrote:
On 1/4/19 6:48 AM, Bernd Edlinger wrote:
This appears to trigger a firmware bug and causes severe
problems with rtl8723ae PCI devices.
When the power save mode is activated for longer periods
of time the firmware stops to receive any packets.
This problem was exposed by commit 873ffe154ae0 ("rtlwifi:
Fix logic error in enter/exit power-save mode").
Previously the power save mode was only active rarely and
only for a short time so that the problem was not noticeable.
Signed-off-by: Bernd Edlinger
---
While the Realtek firmware group has a chance to look for a bug, I would like
you to perform a couple of tests on the original code.
The driver has three module parameters that affect power save. The 'modinfo
rtl8723ae' command lists them as
parm: ips:Set to 0 to not use link power save (default 1) (bool)
parm: swlps:Set to 1 to use SW control power save (default 0) (bool)
parm: fwlps:Set to 1 to use FW control power save (default 1) (bool)
If you were to load rtl8723ae with 'ips=0', does it still fail?
If you were to load the driver with 'swlps=1 fwlps=0', does it still fail?
this does not work:
modprobe rtl8723ae debug_mask=0x debug_level=5 ips=0
tail -f /var/log/syslog|grep "AP off"
Jan 5 11:42:06 w-ed kernel: [ 7267.229713] rtlwifi: :<0> AP off for 2 s
Jan 5 11:42:08 w-ed kernel: [ 7269.276761] rtlwifi: :<0> AP off for 4 s
Jan 5 11:42:10 w-ed kernel: [ 7271.323758] rtlwifi: :<0> AP off for 6 s
Jan 5 11:42:12 w-ed kernel: [ 7273.370759] rtlwifi: :<0> AP off for 8 s
Jan 5 11:42:14 w-ed kernel: [ 7275.417753] rtlwifi: :<0> AP off for 10 s
Jan 5 11:42:14 w-ed kernel: [ 7275.417754] rtlwifi: AP off, try to reconnect
now
Jan 5 11:42:28 w-ed kernel: [ 7289.746676] rtlwifi: :<0> AP off for 2 s
Jan 5 11:42:40 w-ed kernel: [ 7302.028327] rtlwifi: :<0> AP off for 2 s
Jan 5 11:42:43 w-ed kernel: [ 7304.075327] rtlwifi: :<0> AP off for 4 s
Jan 5 11:42:45 w-ed kernel: [ 7306.122330] rtlwifi: :<0> AP off for 6 s
Jan 5 11:42:47 w-ed kernel: [ 7308.169292] rtlwifi: :<0> AP off for 8 s
Jan 5 11:42:49 w-ed kernel: [ 7310.216236] rtlwifi: :<0> AP off for 10 s
Jan 5 11:42:49 w-ed kernel: [ 7310.216238] rtlwifi: AP off, try to reconnect
now
Jan 5 11:43:05 w-ed kernel: [ 7326.59] rtlwifi: :<0> AP off for 2 s
Jan 5 11:43:07 w-ed kernel: [ 7328.639076] rtlwifi: :<0> AP off for 4 s
Jan 5 11:43:09 w-ed kernel: [ 7330.686220] rtlwifi: :<0> AP off for 6 s
Jan 5 11:43:11 w-ed kernel: [ 7332.733078] rtlwifi: :<0> AP off for 8 s
Jan 5 11:43:13 w-ed kernel: [ 7334.779988] rtlwifi: :<0> AP off for 10 s
Jan 5 11:43:13 w-ed kernel: [ 7334.779989] rtlwifi: AP off, try to reconnect
now
Jan 5 11:43:28 w-ed kernel: [ 7349.108839] rtlwifi: :<0> AP off for 2 s
Jan 5 11:43:30 w-ed kernel: [ 7351.155837] rtlwifi: :<0> AP off for 4 s
Jan 5 11:43:32 w-ed kernel: [ 7353.202838] rtlwifi: :<0> AP off for 6 s
Jan 5 11:43:42 w-ed kernel: [ 7363.437779] rtlwifi: :<0> AP off for 2 s
Jan 5 11:43:46 w-ed kernel: [ 7367.531622] rtlwifi: :<0> AP off for 2 s
Jan 5 11:43:48 w-ed kernel: [ 7369.578597] rtlwifi: :<0> AP off for 4 s
Jan 5 11:43:50 w-ed kernel: [ 7371.625694] rtlwifi: :<0> AP off for 6 s
Jan 5 11:43:52 w-ed kernel: [ 7373.672691] rtlwifi: :<0> AP off for 8 s
Jan 5 11:43:54 w-ed kernel: [ 7375.719690] rtlwifi: :<0> AP off for 10 s
Jan 5 11:43:54 w-ed kernel: [ 7375.719691] rtlwifi: AP off, try to reconnect
now
Jan 5 11:44:09 w-ed kernel: [ 7390.048406] rtlwifi: :<0> AP off for 2 s
Jan 5 11:44:11 w-ed kernel: [ 7392.095678] rtlwifi: :<0> AP off for 4 s
Jan 5 11:44:13 w-ed kernel: [ 7394.142349] rtlwifi: :<0> AP off for 6 s
Jan 5 11:44:15 w-ed kernel: [ 7396.189352] rtlwifi: :<0> AP off for 8 s
Jan 5 11:44:17 w-ed kernel: [ 7398.236352] rtlwifi: :<0> AP off for 10 s
Jan 5 11:44:17 w-ed kernel: [ 7398.236353] rtlwifi: AP off, try to reconnect
now
Jan 5 11:44:31 w-ed kernel: [ 7412.565079] rtlwifi: :<0> AP off for 2 s
Jan 5 11:44:33 w-ed kernel: [ 7414.612167] rtlwifi: :<0> AP off for 4 s
Jan 5 11:44:35 w-ed kernel: [ 7416.659101] rtlwifi: :<0> AP off for 6 s
Jan 5 11:44:37 w-ed kernel: [ 7418.706035] rtlwifi: :<0> AP off for 8 s
Jan 5 11:44:39 w-ed kernel: [ 7420.753100] rtlwifi: :<0> AP off for 10 s
Jan 5 11:44:39 w-ed kernel: [ 7420.753101] rtlwifi: AP off, try to reconnect
now
Jan 5 11:44:54 w-ed kernel: [ 7435.081860] rtlwifi: :<0> AP off for 2 s
Jan 5 11:44:56 w-ed kernel: [ 7437.128857] rtlwifi: :<0> AP off for 4 s
Jan 5 11:45:08 w-ed kernel: [ 7449.410653] rtlwifi: :<0> AP off for 2 s
Jan 5 11:45:10 w-ed kernel: [ 7451.457650] rtlwifi: :<0> AP off for 4 s
Jan 5 11:45:12 w-ed kernel: [ 7453.504647] rtlwifi: :<0> AP off for 6
Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
On 1/5/19 3:10 AM, Július Milan wrote:
Fixes the following sparse warnings:
drivers/staging/wilc1000/host_interface.c:2360:30: warning:
incorrect type in assignment (different base types)
expected restricted __le32 [addressable] [assigned] [usertype] frame_type
got restricted __le16 [usertype]
Fixes: 147ccfd451024 ("staging: wilc1000: handle mgmt_frame_register ops from
cfg82011 context")
Signed-off-by: Július Milan
---
drivers/staging/wilc1000/host_interface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 5dae6e7155d3..07c3d6293573 100644
--- a/struct wilc_reg_frame
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2357,7 +2357,7 @@ void wilc_frame_register(struct wilc_vif *vif, u16
frame_type, bool reg)
default:
break;
}
- reg_frame.frame_type = cpu_to_le16(frame_type);
+ reg_frame.frame_type = cpu_to_le32(frame_type);
result = wilc_send_config_pkt(vif, WILC_SET_CFG, , 1,
wilc_get_vif_idx(vif));
if (result)
Before you send V3, are you sure this is the correct fix? As "frame_type" is
input as u16, it seems to me that the frame_type member of struct wilc_reg_frame
should be __le16, not __le32.
Larry
Re: [PATCH 1/4] rtlwifi: rtl8723ae: Take the FW LPS mode handling out
On 1/4/19 6:48 AM, Bernd Edlinger wrote:
This appears to trigger a firmware bug and causes severe
problems with rtl8723ae PCI devices.
When the power save mode is activated for longer periods
of time the firmware stops to receive any packets.
This problem was exposed by commit 873ffe154ae0 ("rtlwifi:
Fix logic error in enter/exit power-save mode").
Previously the power save mode was only active rarely and
only for a short time so that the problem was not noticeable.
Signed-off-by: Bernd Edlinger
---
While the Realtek firmware group has a chance to look for a bug, I would like
you to perform a couple of tests on the original code.
The driver has three module parameters that affect power save. The 'modinfo
rtl8723ae' command lists them as
parm: ips:Set to 0 to not use link power save (default 1) (bool)
parm: swlps:Set to 1 to use SW control power save (default 0) (bool)
parm: fwlps:Set to 1 to use FW control power save (default 1) (bool)
If you were to load rtl8723ae with 'ips=0', does it still fail?
If you were to load the driver with 'swlps=1 fwlps=0', does it still fail?
Thanks for debugging this problem.
Larry
Re: [PATCH] Revert "staging:r8188eu: use lib80211 CCMP decrypt"
On 1/1/19 1:31 PM, Michael Straube wrote: I've tested your patch and it solved the issue. No freezes and dmesg looks good. I noticed that try_then_request_module() is also used in rtw_wep_encrypt() and rtw_wep_decrypt(). I guess that also could cause problems? Yes, I believe it would if anyone is still using WEP. My plan is to get rid of the try_then_request_module() there as well, and for completeness, I plan to restore usage of the lib80211 routines for TKIP as well. Once I get a chance to test the TKIP and WEP changes, I plan to have a set of 4 patches to switch the driver to using lib80211 routines for all decryption/encryption. Larry
