* [PATCH 10/12] mwifiex: sdio: don't check for NULL sdio_func
From: Xinming Hu @ 2016-10-31 8:02 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1477900940-10549-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
sdio_func is retrieved via container_of() and should never be NULL.
Checking for NULL just makes the logic more confusing than necessary.
Stop doing that.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 40 +++++++++++------------------
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 375d0a5..8f0f072 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -231,15 +231,10 @@ static int mwifiex_sdio_resume(struct device *dev)
struct mwifiex_adapter *adapter;
mmc_pm_flag_t pm_flag = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- card = sdio_get_drvdata(func);
- if (!card || !card->adapter) {
- pr_err("resume: invalid card or adapter\n");
- return 0;
- }
- } else {
- pr_err("resume: sdio_func is not specified\n");
+ pm_flag = sdio_get_host_pm_caps(func);
+ card = sdio_get_drvdata(func);
+ if (!card || !card->adapter) {
+ dev_err(dev, "resume: invalid card or adapter\n");
return 0;
}
@@ -320,23 +315,18 @@ static int mwifiex_sdio_suspend(struct device *dev)
mmc_pm_flag_t pm_flag = 0;
int ret = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
- sdio_func_id(func), pm_flag);
- if (!(pm_flag & MMC_PM_KEEP_POWER)) {
- pr_err("%s: cannot remain alive while host is"
- " suspended\n", sdio_func_id(func));
- return -ENOSYS;
- }
+ pm_flag = sdio_get_host_pm_caps(func);
+ pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
+ sdio_func_id(func), pm_flag);
+ if (!(pm_flag & MMC_PM_KEEP_POWER)) {
+ dev_err(dev, "%s: cannot remain alive while host is"
+ " suspended\n", sdio_func_id(func));
+ return -ENOSYS;
+ }
- card = sdio_get_drvdata(func);
- if (!card) {
- dev_err(dev, "suspend: invalid card\n");
- return 0;
- }
- } else {
- pr_err("suspend: sdio_func is not specified\n");
+ card = sdio_get_drvdata(func);
+ if (!card) {
+ dev_err(dev, "suspend: invalid card\n");
return 0;
}
--
1.8.1.4
^ permalink raw reply related
* [PATCH 11/12] mwifiex: stop checking for NULL drvata/intfdata
From: Xinming Hu @ 2016-10-31 8:02 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1477900940-10549-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
These are never NULL, so stop making people think they might be.
I don't change this for SDIO because SDIO has a racy card-reset handler
that reallocates this struct. I'd rather not touch that mess right now.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 14 +++++---------
drivers/net/wireless/marvell/mwifiex/usb.c | 15 +++------------
2 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 04b9961..c061d00 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -102,10 +102,6 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card) {
- dev_err(dev, "adapter structure is not valid\n");
- return 0;
- }
/* Might still be loading firmware */
wait_for_completion(&card->fw_done);
@@ -148,8 +144,9 @@ static int mwifiex_pcie_resume(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- dev_err(dev, "Card or adapter structure is not valid\n");
+
+ if (!card->adapter) {
+ dev_err(dev, "adapter structure is not valid\n");
return 0;
}
@@ -222,8 +219,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
struct mwifiex_private *priv;
card = pci_get_drvdata(pdev);
- if (!card)
- return;
wait_for_completion(&card->fw_done);
@@ -2216,7 +2211,8 @@ static irqreturn_t mwifiex_pcie_interrupt(int irq, void *context)
}
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
+
+ if (!card->adapter) {
pr_err("info: %s: card=%p adapter=%p\n", __func__, card,
card ? card->adapter : NULL);
goto exit;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index c26daf4..78b46fa 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -503,11 +503,6 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
struct usb_tx_data_port *port;
int i, j;
- if (!card) {
- dev_err(&intf->dev, "%s: card is NULL\n", __func__);
- return 0;
- }
-
/* Might still be loading firmware */
wait_for_completion(&card->fw_done);
@@ -574,8 +569,9 @@ static int mwifiex_usb_resume(struct usb_interface *intf)
struct mwifiex_adapter *adapter;
int i;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card->adapter) {
+ dev_err(&intf->dev, "%s: card->adapter is NULL\n",
+ __func__);
return 0;
}
adapter = card->adapter;
@@ -617,11 +613,6 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
struct usb_card_rec *card = usb_get_intfdata(intf);
struct mwifiex_adapter *adapter;
- if (!card) {
- dev_err(&intf->dev, "%s: card is NULL\n", __func__);
- return;
- }
-
wait_for_completion(&card->fw_done);
adapter = card->adapter;
--
1.8.1.4
^ permalink raw reply related
* [PATCH 12/12] mwifiex: pcie: stop checking for NULL adapter->card
From: Xinming Hu @ 2016-10-31 8:02 UTC (permalink / raw)
To: Linux Wireless
Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Brian Norris
In-Reply-To: <1477900940-10549-1-git-send-email-huxinming820@marvell.com>
From: Brian Norris <briannorris@chromium.org>
It should never be NULL here, and to think otherwise makes things
confusing.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 55 +++++++++++++----------------
1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index c061d00..86e8ce6 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2990,31 +2990,28 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = card->dev;
int i;
- if (card) {
- pdev = card->dev;
- if (card->msix_enable) {
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- synchronize_irq(card->msix_entries[i].vector);
+ if (card->msix_enable) {
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ synchronize_irq(card->msix_entries[i].vector);
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- free_irq(card->msix_entries[i].vector,
- &card->msix_ctx[i]);
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ free_irq(card->msix_entries[i].vector,
+ &card->msix_ctx[i]);
- card->msix_enable = 0;
- pci_disable_msix(pdev);
- } else {
- mwifiex_dbg(adapter, INFO,
- "%s(): calling free_irq()\n", __func__);
- free_irq(card->dev->irq, &card->share_irq_ctx);
+ card->msix_enable = 0;
+ pci_disable_msix(pdev);
+ } else {
+ mwifiex_dbg(adapter, INFO,
+ "%s(): calling free_irq()\n", __func__);
+ free_irq(card->dev->irq, &card->share_irq_ctx);
- if (card->msi_enable)
- pci_disable_msi(pdev);
- }
- card->adapter = NULL;
- }
+ if (card->msi_enable)
+ pci_disable_msi(pdev);
+ }
+ card->adapter = NULL;
}
/* This function initializes the PCI-E host memory space, WCB rings, etc.
@@ -3097,18 +3094,14 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
adapter->seq_num = 0;
adapter->tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_4K;
- if (card) {
- if (reg->sleep_cookie)
- mwifiex_pcie_delete_sleep_cookie_buf(adapter);
-
- mwifiex_pcie_delete_cmdrsp_buf(adapter);
- mwifiex_pcie_delete_evtbd_ring(adapter);
- mwifiex_pcie_delete_rxbd_ring(adapter);
- mwifiex_pcie_delete_txbd_ring(adapter);
- card->cmdrsp_buf = NULL;
- }
+ if (reg->sleep_cookie)
+ mwifiex_pcie_delete_sleep_cookie_buf(adapter);
- return;
+ mwifiex_pcie_delete_cmdrsp_buf(adapter);
+ mwifiex_pcie_delete_evtbd_ring(adapter);
+ mwifiex_pcie_delete_rxbd_ring(adapter);
+ mwifiex_pcie_delete_txbd_ring(adapter);
+ card->cmdrsp_buf = NULL;
}
static struct mwifiex_if_ops pcie_ops = {
--
1.8.1.4
^ permalink raw reply related
* RE: [PATCH v4 1/3] mwifiex: reset card->adapter during device unregister
From: Amitkumar Karwar @ 2016-10-31 10:33 UTC (permalink / raw)
To: Brian Norris
Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
rajatja@google.com, Xinming Hu, abhishekbh@google.com,
Dmitry Torokhov
In-Reply-To: <20161025165429.GA64548@google.com>
Hi Kalle,
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Tuesday, October 25, 2016 10:25 PM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> rajatja@google.com; Xinming Hu; abhishekbh@google.com; Dmitry Torokhov
> Subject: Re: [PATCH v4 1/3] mwifiex: reset card->adapter during device
> unregister
>
Please ignore this patch series (and also v5)
As Brian pointed out, it doesn't address race issues correctly.
Xinming Hu has submitted a patch series(having 12 patches) from Brian which resolves the issues and performs some cleanup work.
Regards,
Amitkumar Karwar.
^ permalink raw reply
* [PATCH v7 1/2] nl80211: multicast_to_unicast can be changed while IFF_UP
From: Michael Braun @ 2016-10-31 13:40 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, netdev, projekt-wlan
There is no need to prevent toggling multicast_to_unicast while
interface is already up. This change simplifies reconfiguration
from hostapd.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/wireless/nl80211.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5349c63..7554400 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -11006,9 +11006,6 @@ static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
const struct nlattr *nla;
bool enabled;
- if (netif_running(dev))
- return -EBUSY;
-
if (!rdev->ops->set_multicast_to_unicast)
return -EOPNOTSUPP;
--
2.1.4
^ permalink raw reply related
* [PATCH v7 2/2] mac80211: multicast to unicast conversion
From: Michael Braun @ 2016-10-31 13:41 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless, netdev, projekt-wlan
In-Reply-To: <1477921260-24556-1-git-send-email-michael-dev@fami-braun.de>
Add the ability for an AP (and associated VLANs) to perform
multicast-to-unicast conversion for ARP, IPv4 and IPv6 frames
(possibly within 802.1Q). If enabled, such frames are to be sent
to each station separately, with the DA replaced by their own
MAC address rather than the group address.
Note that this may break certain expectations of the receiver,
such as the ability to drop unicast IP packets received within
multicast L2 frames, or the ability to not send ICMP destination
unreachable messages for packets received in L2 multicast (which
is required, but the receiver can't tell the difference if this
new option is enabled.)
This also doesn't implement the 802.11 DMS (directed multicast
service).
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
--
v7:
- avoid recursion
- style and description
v5:
- rename bss->unicast to bss->multicast_to_unicast
- access sdata->bss only after checking iftype
v4:
- rename MULTICAST_TO_UNICAST to MULTICAST_TO_UNICAST
v3: fix compile error for trace.h
v2: add nl80211 toggle
rename tx_dnat to change_da
change int to bool unicast
---
net/mac80211/cfg.c | 16 ++++++
net/mac80211/debugfs_netdev.c | 3 ++
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/tx.c | 122 +++++++++++++++++++++++++++++++++++++++++-
4 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1edb017..a861d85 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3345,6 +3345,21 @@ static int ieee80211_del_tx_ts(struct wiphy *wiphy, struct net_device *dev,
return -ENOENT;
}
+static int ieee80211_set_multicast_to_unicast(struct wiphy *wiphy,
+ struct net_device *dev,
+ const bool enabled)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+ /* not supported with P2P_GO for now */
+ if (sdata->vif.type != NL80211_IFTYPE_AP)
+ return -EOPNOTSUPP;
+
+ sdata->u.ap.multicast_to_unicast = enabled;
+
+ return 0;
+}
+
const struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -3430,4 +3445,5 @@ const struct cfg80211_ops mac80211_config_ops = {
.set_ap_chanwidth = ieee80211_set_ap_chanwidth,
.add_tx_ts = ieee80211_add_tx_ts,
.del_tx_ts = ieee80211_del_tx_ts,
+ .set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
};
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index ed7bff4..509c6c3 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -487,6 +487,8 @@ static ssize_t ieee80211_if_fmt_num_buffered_multicast(
}
IEEE80211_IF_FILE_R(num_buffered_multicast);
+IEEE80211_IF_FILE(multicast_to_unicast, u.ap.multicast_to_unicast, HEX);
+
/* IBSS attributes */
static ssize_t ieee80211_if_fmt_tsf(
const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -642,6 +644,7 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
DEBUGFS_ADD(dtim_count);
DEBUGFS_ADD(num_buffered_multicast);
DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
+ DEBUGFS_ADD_MODE(multicast_to_unicast, 0600);
}
static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 70c0963..84374ed 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -293,6 +293,7 @@ struct ieee80211_if_ap {
driver_smps_mode; /* smps mode request */
struct work_struct request_smps_work;
+ bool multicast_to_unicast;
};
struct ieee80211_if_wds {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c3ce86e..7271c93 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/skbuff.h>
+#include <linux/if_vlan.h>
#include <linux/etherdevice.h>
#include <linux/bitmap.h>
#include <linux/rcupdate.h>
@@ -3418,6 +3419,115 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
rcu_read_unlock();
}
+static int ieee80211_change_da(struct sk_buff *skb, struct sta_info *sta)
+{
+ struct ethhdr *eth;
+ int err;
+
+ err = skb_ensure_writable(skb, ETH_HLEN);
+ if (unlikely(err))
+ return err;
+
+ eth = (void *)skb->data;
+ ether_addr_copy(eth->h_dest, sta->sta.addr);
+
+ return 0;
+}
+
+static inline int
+ieee80211_multicast_to_unicast(struct sk_buff *skb, struct net_device *dev)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ const struct ethhdr *eth = (void *)skb->data;
+ const struct vlan_ethhdr *ethvlan = (void *)skb->data;
+ u16 ethertype;
+
+ if (likely(!is_multicast_ether_addr(eth->h_dest)))
+ return 0;
+
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP_VLAN:
+ if (sdata->u.vlan.sta)
+ return 0;
+ if (sdata->wdev.use_4addr)
+ return 0;
+ /* fall through */
+ case NL80211_IFTYPE_AP:
+ /* check runtime toggle for this bss */
+ if (!sdata->bss->multicast_to_unicast)
+ return 0;
+ break;
+ default:
+ return 0;
+ }
+
+ /* multicast to unicast conversion only for some payload */
+ ethertype = ntohs(eth->h_proto);
+ if (ethertype == ETH_P_8021Q && skb->len >= VLAN_ETH_HLEN)
+ ethertype = ntohs(ethvlan->h_vlan_encapsulated_proto);
+ switch (ethertype) {
+ case ETH_P_ARP:
+ case ETH_P_IP:
+ case ETH_P_IPV6:
+ break;
+ default:
+ return 0;
+ }
+
+ return 1;
+}
+
+static void
+ieee80211_convert_to_unicast(struct sk_buff *skb, struct net_device *dev,
+ struct sk_buff_head *queue)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
+ const struct ethhdr *eth = (struct ethhdr *)skb->data;
+ struct sta_info *sta, *first = NULL;
+ struct sk_buff *cloned_skb;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ if (sdata != sta->sdata)
+ /* AP-VLAN mismatch */
+ continue;
+ if (unlikely(ether_addr_equal(eth->h_source, sta->sta.addr)))
+ /* do not send back to source */
+ continue;
+ if (!first) {
+ first = sta;
+ continue;
+ }
+ cloned_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!cloned_skb)
+ goto unicast;
+ if (unlikely(ieee80211_change_da(cloned_skb, sta))) {
+ dev_kfree_skb(cloned_skb);
+ goto unicast;
+ }
+ __skb_queue_tail(queue, cloned_skb);
+ }
+
+ if (likely(first)) {
+ if (unlikely(ieee80211_change_da(skb, first)))
+ goto unicast;
+ __skb_queue_tail(queue, skb);
+ } else {
+ /* no STA connected, drop */
+ kfree_skb(skb);
+ skb = NULL;
+ }
+
+ goto out;
+unicast:
+ __skb_queue_purge(queue);
+ __skb_queue_tail(queue, skb);
+out:
+ rcu_read_unlock();
+}
+
/**
* ieee80211_subif_start_xmit - netif start_xmit function for 802.3 vifs
* @skb: packet to be sent
@@ -3428,7 +3538,17 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
- __ieee80211_subif_start_xmit(skb, dev, 0);
+ if (unlikely(ieee80211_multicast_to_unicast(skb, dev))) {
+ struct sk_buff_head queue;
+
+ __skb_queue_head_init(&queue);
+ ieee80211_convert_to_unicast(skb, dev, &queue);
+ while ((skb = __skb_dequeue(&queue)))
+ __ieee80211_subif_start_xmit(skb, dev, 0);
+ } else {
+ __ieee80211_subif_start_xmit(skb, dev, 0);
+ }
+
return NETDEV_TX_OK;
}
--
2.1.4
^ permalink raw reply related
* [PATCH] ath9k_htc: fix minor mistakes in dev_err messages
From: Colin King @ 2016-10-31 15:12 UTC (permalink / raw)
To: QCA ath9k Development, Kalle Valo, linux-wireless, ath9k-devel,
netdev
Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Add missing space in a dev_err message and join wrapped text so
it does not span multiple lines. Fix spelling mistake on "unknown".
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/ath/ath9k/htc_hst.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index fd85f99..8e6dae2 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -244,8 +244,8 @@ int htc_connect_service(struct htc_target *target,
/* Find an available endpoint */
endpoint = get_next_avail_ep(target->endpoint);
if (!endpoint) {
- dev_err(target->dev, "Endpoint is not available for"
- "service %d\n", service_connreq->service_id);
+ dev_err(target->dev, "Endpoint is not available for service %d\n",
+ service_connreq->service_id);
return -EINVAL;
}
@@ -382,7 +382,7 @@ static void ath9k_htc_fw_panic_report(struct htc_target *htc_handle,
break;
}
default:
- dev_err(htc_handle->dev, "ath: uknown panic pattern!\n");
+ dev_err(htc_handle->dev, "ath: unknown panic pattern!\n");
break;
}
}
--
2.9.3
^ permalink raw reply related
* [char-misc v4.9] mei: bus: fix received data size check in NFC fixup
From: Tomas Winkler @ 2016-10-31 17:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Alexander Usyskin, linux-kernel, Lauro Ramos Venancio,
Aloisio Almeida Jr, Samuel Ortiz, linux-wireless, Tomas Winkler
From: Alexander Usyskin <alexander.usyskin@intel.com>
NFC version reply size checked against only header size, not against
full message size. That may lead potentially to uninitialized memory access
in version data.
That leads to warnings when version data is accessed:
drivers/misc/mei/bus-fixup.c: warning: '*((void *)&ver+11)' may be used uninitialized in this function [-Wuninitialized]: => 212:2
Reported in
Build regressions/improvements in v4.9-rc3
https://lkml.org/lkml/2016/10/30/57
Fixes: 59fcd7c63abf (mei: nfc: Initial nfc implementation)
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/misc/mei/bus-fixup.c | 2 +-
drivers/nfc/mei_phy.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index e9e6ea3ab73c..75b9d4ac8b1e 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -178,7 +178,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
ret = 0;
bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
- if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
+ if (bytes_recv < if_version_length) {
dev_err(bus->dev, "Could not read IF version\n");
ret = -EIO;
goto err;
diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 07b4239585fa..03139c5a05e4 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -133,7 +133,7 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
return -ENOMEM;
bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, if_version_length);
- if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
+ if (bytes_recv < 0 || bytes_recv < if_version_length) {
pr_err("Could not read IF version\n");
r = -EIO;
goto err;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Rajat Jain @ 2016-10-31 17:09 UTC (permalink / raw)
To: Rob Herring
Cc: Rajat Jain, linux-wireless, devicetree, Xinming Hu,
Amitkumar Karwar, Brian Norris, Kalle Valo
In-Reply-To: <20161030204131.pwfjqekpwd6nr3gb@rob-hp-laptop>
On Sun, Oct 30, 2016 at 1:41 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
>> From: Xinming Hu <huxm@marvell.com>
>>
>> This patch derives device tree node from pcie bus layer framework, and
>> fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
>> Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
>> marvell-8xxx.txt) to accommodate PCIe changes.
>>
>> Signed-off-by: Xinming Hu <huxm@marvell.com>
>> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
>> Signed-off-by: Rajat Jain <rajatja@google.com>
>> Reviewed-by: Brian Norris <briannorris@chromium.org>
>> ---
>> v2: Included vendor and product IDs in compatible strings for PCIe
>> chipsets(Rob Herring)
>> v3: Patch is created using -M option so that it will only include diff of
>> original and renamed files(Rob Herring)
>> Resend v3: Resending the patch because I missed to include device tree mailing
>> while sending v3.
>> v4: Fix error handling, also move-on even if no device tree node is present.
>> v5: Update commit log to include memory leak, return -EINVAL instead of -1.
>> v6: Remove an unnecessary error print, fix typo in commit log
>>
>> .../{marvell-sd8xxx.txt => marvell-8xxx.txt} | 8 +++--
>> drivers/net/wireless/marvell/mwifiex/pcie.c | 36 +++++++++++++++++++---
>> drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 3 +-
>> 3 files changed, 39 insertions(+), 8 deletions(-)
>> rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => marvell-8xxx.txt} (91%)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
>> similarity index 91%
>> rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>> rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
>> index c421aba..dfe5f8e 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
>> @@ -1,8 +1,8 @@
>> -Marvell 8897/8997 (sd8897/sd8997) SDIO devices
>> +Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
>> ------
>>
>> -This node provides properties for controlling the marvell sdio wireless device.
>> -The node is expected to be specified as a child node to the SDIO controller that
>> +This node provides properties for controlling the marvell sdio/pcie wireless device.
>
> s/marvell/Marvell/
> s/sdio\/pcie/SDIO\/PCIE/
>
>> +The node is expected to be specified as a child node to the SDIO/PCIE controller that
>> connects the device to the system.
>>
>> Required properties:
>> @@ -10,6 +10,8 @@ Required properties:
>> - compatible : should be one of the following:
>> * "marvell,sd8897"
>> * "marvell,sd8997"
>> + * "pci11ab,2b42"
>> + * "pci1b4b,2b42"
>
Hi Rob,
> I think I already said this, but you have the vendor and product IDs
> reversed.
Actually Marvell has 2 vendor IDs assigned to it. In include/linux/pci_ids.h:
#define PCI_VENDOR_ID_MARVELL 0x11ab
#define PCI_VENDOR_ID_MARVELL_EXT 0x1b4b
So in this case the compatible property describes a single product ID,
with both possible vendor IDs.
Thanks,
Rajat
>
> Rob
^ permalink raw reply
* [PATCH RESEND v2] cfg80211: add bitrate for 20MHz MCS 9
From: Thomas Pedersen @ 2016-10-31 18:28 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Thomas Pedersen
Some drivers (ath10k) report MCS 9 @ 20MHz, which
technically isn't defined. To get more meaningful value
than 0 out of this however, just extrapolate a bitrate
from ratio of MCS 7 and 9 in channels where it is allowed.
Signed-off-by: Thomas Pedersen <twp@qca.qualcomm.com>
---
v2: add MCS 9 bitrate instead of capping at MCS 8
---
net/wireless/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 32060f8..30fc320 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1163,7 +1163,7 @@ static u32 cfg80211_calculate_bitrate_vht(struct rate_info *rate)
58500000,
65000000,
78000000,
- 0,
+ 86500000,
},
{ 13500000,
27000000,
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-10-31 18:35 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless, netdev; +Cc: linux-kernel
The rtl8723bu wireless IC shows evidence of a more agressive approach to
power saving, powering down its RF side when there is no wireless
interfacing but leaving USB interfacing intact. This makes the wireless
IC more suitable for use in devices which need to keep their power use
as low as practical, such as tablets and Surface Pro type devices.
In effect this means that a full initialisation must be performed
whenever a wireless interface is brought up. It also means that
interpretations of power status from general wireless registers should
not be relied on to influence an init sequence.
The patch works by forcing a fuller initialisation and forcing it to
occur more often in code paths (such as occurs during a low level
authentication that initiates wireless interfacing).
The initialisation sequence is now more consistent with code based
directly on vendor code. For example while the vendor derived code
interprets a register as indcating a particular powered state, it does
not use this information to influence its init sequence.
The rtl8723bu device has a unique USB VID and PID. This is taken
advantage of for the patch to ensure only rtl8723bu devices are affected
by this patch.
With this patch wpa_supplicant reliably and consistently connects with
an AP. Before a workaround such as executing rmmod and modprobe before
each call to wpa_supplicant worked with some distributions.
Signed-off-by: John Heenan <john@zgus.com>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 24 ++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 04141e5..f36e674 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -79,6 +79,8 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
#define RTL8XXXU_TX_URB_LOW_WATER 25
#define RTL8XXXU_TX_URB_HIGH_WATER 32
+#define USB_PRODUCT_ID_RTL8723BU 0xb720
+
static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
struct rtl8xxxu_rx_urb *rx_urb);
@@ -3892,6 +3894,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
u8 val8;
u16 val16;
u32 val32;
+ struct usb_device_descriptor *udesc = &priv->udev->descriptor;
/* Check if MAC is already powered on */
val8 = rtl8xxxu_read8(priv, REG_CR);
@@ -3900,7 +3903,9 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
* Fix 92DU-VC S3 hang with the reason is that secondary mac is not
* initialized. First MAC returns 0xea, second MAC returns 0x00
*/
- if (val8 == 0xea)
+ if (val8 == 0xea
+ || (udesc->idVendor == USB_VENDOR_ID_REALTEK
+ && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU))
macpower = false;
else
macpower = true;
@@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
struct rtl8xxxu_tx_urb *tx_urb;
unsigned long flags;
int ret, i;
+ struct usb_device_descriptor *udesc = &priv->udev->descriptor;
ret = 0;
+ if(udesc->idVendor == USB_VENDOR_ID_REALTEK
+ && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
+ ret = rtl8xxxu_init_device(hw);
+ if (ret)
+ goto error_out;
+ }
+
init_usb_anchor(&priv->rx_anchor);
init_usb_anchor(&priv->tx_anchor);
init_usb_anchor(&priv->int_anchor);
@@ -6080,9 +6093,12 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
goto exit;
}
- ret = rtl8xxxu_init_device(hw);
- if (ret)
+ if(!(id->idVendor == USB_VENDOR_ID_REALTEK
+ && id->idProduct == USB_PRODUCT_ID_RTL8723BU)) {
+ ret = rtl8xxxu_init_device(hw);
+ if (ret)
goto exit;
+ }
hw->wiphy->max_scan_ssids = 1;
hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
@@ -6191,7 +6207,7 @@ static struct usb_device_id dev_table[] = {
/* Tested by Myckel Habets */
{USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0109, 0xff, 0xff, 0xff),
.driver_info = (unsigned long)&rtl8192eu_fops},
-{USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0xb720, 0xff, 0xff, 0xff),
+{USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, USB_PRODUCT_ID_RTL8723BU, 0xff, 0xff, 0xff),
.driver_info = (unsigned long)&rtl8723bu_fops},
#ifdef CONFIG_RTL8XXXU_UNTESTED
/* Still supported by rtlwifi */
--
2.10.1
^ permalink raw reply related
* Re: Backwards 11ac
From: James Cloos @ 2016-10-31 18:35 UTC (permalink / raw)
To: linux-wireless; +Cc: Sebastian Gottschall
In-Reply-To: <f8094de7-1357-a9d0-fee3-b28e06dd3bc7@dd-wrt.com>
>>>>> "SG" == Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
SG> ath10k cannot show tx rates right now in a easy way. since tx rate
SG> handling is done by the cards own firmware and not by the mac80211
SG> wireless stack
Thanks for that info, but there is still the issue that download
bandwidth is limited to around 6 Mbps whereas upload bandwidth is around
50 Mbps.
-JimC
--
James Cloos <cloos@jhcloos.com> OpenPGP: 0x997A9F17ED7DAEA6
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2016-10-30
From: David Miller @ 2016-10-31 19:38 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87insaw5u9.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Sun, 30 Oct 2016 11:20:46 +0200
> few fixes for 4.9. I tagged this on the plane over a slow mosh
> connection while travelling to Plumbers so I might have done something
> wrong, please check more carefully than usually. For example I had to
> redo the signed tag because of some whitespace damage.
>
> Please let me know if there are any problems.
Your subject line says "-next" but clearly these are bug fixes for 'net'
so that's where I pulled this into.
Thanks.
^ permalink raw reply
* Re: Backwards 11ac
From: James Cloos @ 2016-10-31 21:02 UTC (permalink / raw)
To: linux-wireless; +Cc: Sebastian Gottschall
In-Reply-To: <89dd7bd2-f6de-ea90-79ee-65d73d264522@dd-wrt.com>
>>>>> "SG" == Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
JC>> Thanks for that info, but there is still the issue that download
JC>> bandwidth is limited to around 6 Mbps whereas upload bandwidth is around
JC>> 50 Mbps
SG> thats unusual. i do not suffer from this issue with the latest driver
SG> code and also never had it. but as i said. the visual tx rate doesnt
SG> matter and isnt a reason for any issue here.
SG> its just a number
Those numbers above are actual throughput, tested via rsync/ssh. Even
adding in the rsync, ssh, tcp and ip overheads that is still worse than
the 11n-40 on the 2.4 band. And of course backwards.
Does anyone have any explanation for the backwards bandwidth or solution
for how to fix it?
-JimC
--
James Cloos <cloos@jhcloos.com> OpenPGP: 0x997A9F17ED7DAEA6
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: Jes Sorensen @ 2016-10-31 21:25 UTC (permalink / raw)
To: John Heenan; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <20161031183522.GA13315@cube>
John Heenan <john@zgus.com> writes:
> The rtl8723bu wireless IC shows evidence of a more agressive approach to
> power saving, powering down its RF side when there is no wireless
> interfacing but leaving USB interfacing intact. This makes the wireless
> IC more suitable for use in devices which need to keep their power use
> as low as practical, such as tablets and Surface Pro type devices.
>
> In effect this means that a full initialisation must be performed
> whenever a wireless interface is brought up. It also means that
> interpretations of power status from general wireless registers should
> not be relied on to influence an init sequence.
>
> The patch works by forcing a fuller initialisation and forcing it to
> occur more often in code paths (such as occurs during a low level
> authentication that initiates wireless interfacing).
>
> The initialisation sequence is now more consistent with code based
> directly on vendor code. For example while the vendor derived code
> interprets a register as indcating a particular powered state, it does
> not use this information to influence its init sequence.
>
> The rtl8723bu device has a unique USB VID and PID. This is taken
> advantage of for the patch to ensure only rtl8723bu devices are affected
> by this patch.
>
> With this patch wpa_supplicant reliably and consistently connects with
> an AP. Before a workaround such as executing rmmod and modprobe before
> each call to wpa_supplicant worked with some distributions.
>
> Signed-off-by: John Heenan <john@zgus.com>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 24 ++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e5..f36e674 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -79,6 +79,8 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
> #define RTL8XXXU_TX_URB_LOW_WATER 25
> #define RTL8XXXU_TX_URB_HIGH_WATER 32
>
> +#define USB_PRODUCT_ID_RTL8723BU 0xb720
> +
Absolutely not! You have no guarantee that this is the only id used for
8723bu devices, and adding a #define for each is not going to happen.
> static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
> struct rtl8xxxu_rx_urb *rx_urb);
>
> @@ -3892,6 +3894,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> u8 val8;
> u16 val16;
> u32 val32;
> + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
Indentaiton
> /* Check if MAC is already powered on */
> val8 = rtl8xxxu_read8(priv, REG_CR);
> @@ -3900,7 +3903,9 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
> * initialized. First MAC returns 0xea, second MAC returns 0x00
> */
> - if (val8 == 0xea)
> + if (val8 == 0xea
> + || (udesc->idVendor == USB_VENDOR_ID_REALTEK
> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU))
> macpower = false;
> else
> macpower = true;
Please respect proper kernel coding style!
> @@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
> struct rtl8xxxu_tx_urb *tx_urb;
> unsigned long flags;
> int ret, i;
> + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
>
> ret = 0;
>
> + if(udesc->idVendor == USB_VENDOR_ID_REALTEK
> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
> + goto error_out;
> + }
> +
As mentioned previously, if this is to be changed here, it has to be
matched in the _stop section too. It also has to be investigated exactly
why this matters for 8723bu. It is possible this matters for other
devices, but we need to find out exactly what causes this not moving a
major block of init code around like this.
> init_usb_anchor(&priv->rx_anchor);
> init_usb_anchor(&priv->tx_anchor);
> init_usb_anchor(&priv->int_anchor);
> @@ -6080,9 +6093,12 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> goto exit;
> }
>
> - ret = rtl8xxxu_init_device(hw);
> - if (ret)
> + if(!(id->idVendor == USB_VENDOR_ID_REALTEK
> + && id->idProduct == USB_PRODUCT_ID_RTL8723BU)) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
> goto exit;
> + }
Again, this coding style abuse will never go into this driver,
Jes
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-10-31 22:15 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <wrfj1sywrz2f.fsf@redhat.com>
On 1 November 2016 at 07:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> John Heenan <john@zgus.com> writes:
>> The rtl8723bu wireless IC shows evidence of a more agressive approach to
>> power saving, powering down its RF side when there is no wireless
>> interfacing but leaving USB interfacing intact. This makes the wireless
>> IC more suitable for use in devices which need to keep their power use
>> as low as practical, such as tablets and Surface Pro type devices.
>>
>> In effect this means that a full initialisation must be performed
>> whenever a wireless interface is brought up. It also means that
>> interpretations of power status from general wireless registers should
>> not be relied on to influence an init sequence.
>>
>> The patch works by forcing a fuller initialisation and forcing it to
>> occur more often in code paths (such as occurs during a low level
>> authentication that initiates wireless interfacing).
>>
>> The initialisation sequence is now more consistent with code based
>> directly on vendor code. For example while the vendor derived code
>> interprets a register as indcating a particular powered state, it does
>> not use this information to influence its init sequence.
>>
>> The rtl8723bu device has a unique USB VID and PID. This is taken
>> advantage of for the patch to ensure only rtl8723bu devices are affected
>> by this patch.
>>
>> With this patch wpa_supplicant reliably and consistently connects with
>> an AP. Before a workaround such as executing rmmod and modprobe before
>> each call to wpa_supplicant worked with some distributions.
>>
>> Signed-off-by: John Heenan <john@zgus.com>
>> ---
>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 24 ++++++++++++++++++----
>> 1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 04141e5..f36e674 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -79,6 +79,8 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
>> #define RTL8XXXU_TX_URB_LOW_WATER 25
>> #define RTL8XXXU_TX_URB_HIGH_WATER 32
>>
>> +#define USB_PRODUCT_ID_RTL8723BU 0xb720
>> +
>
> Absolutely not! You have no guarantee that this is the only id used for
> 8723bu devices, and adding a #define for each is not going to happen.
Thanks for you reply.
I have no problem with that. However the patch does get the point
across in a minimalist and efficient way of what the issues are.
Currently there is no property available to determine the information required.
>
>> static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
>> struct rtl8xxxu_rx_urb *rx_urb);
>>
>> @@ -3892,6 +3894,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>> u8 val8;
>> u16 val16;
>> u32 val32;
>> + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
>
> Indentaiton
OK. Missed that one.
>
>> /* Check if MAC is already powered on */
>> val8 = rtl8xxxu_read8(priv, REG_CR);
>> @@ -3900,7 +3903,9 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>> * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>> * initialized. First MAC returns 0xea, second MAC returns 0x00
>> */
>> - if (val8 == 0xea)
>> + if (val8 == 0xea
>> + || (udesc->idVendor == USB_VENDOR_ID_REALTEK
>> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU))
>> macpower = false;
>> else
>> macpower = true;
>
> Please respect proper kernel coding style!
I don't know what you mean. Your code has real tabs. My code has real
tabs. The kernel style goes on about tabs being 8 spaces. So do you
want: real tabs or real spaces?
You said no lines over 80 columns long. This is what i have done.
>
>> @@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>> struct rtl8xxxu_tx_urb *tx_urb;
>> unsigned long flags;
>> int ret, i;
>> + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
>>
>> ret = 0;
>>
>> + if(udesc->idVendor == USB_VENDOR_ID_REALTEK
>> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
>> + ret = rtl8xxxu_init_device(hw);
>> + if (ret)
>> + goto error_out;
>> + }
>> +
>
> As mentioned previously, if this is to be changed here, it has to be
> matched in the _stop section too.
I looked at this and could not find a matching function. I will have a
look again.
> It also has to be investigated exactly
> why this matters for 8723bu. It is possible this matters for other
> devices, but we need to find out exactly what causes this not moving a
> major block of init code around like this.
I have no problem with this.
Given what you said yesterday we are not going to get definitive
answers. It is going to have to be trial and error while tracing
through what vendor code does.
Apparently as far as Realtek are concerned, we are nobodies and they
want nothing to do with us.
>
>> init_usb_anchor(&priv->rx_anchor);
>> init_usb_anchor(&priv->tx_anchor);
>> init_usb_anchor(&priv->int_anchor);
>> @@ -6080,9 +6093,12 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>> goto exit;
>> }
>>
>> - ret = rtl8xxxu_init_device(hw);
>> - if (ret)
>> + if(!(id->idVendor == USB_VENDOR_ID_REALTEK
>> + && id->idProduct == USB_PRODUCT_ID_RTL8723BU)) {
>> + ret = rtl8xxxu_init_device(hw);
>> + if (ret)
>> goto exit;
>> + }
>
> Again, this coding style abuse will never go into this driver,
As above, what abuse? I am not being facetious. Just puzzled.
> Jes
Again have a nice day!
John
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: Barry Day @ 2016-10-31 22:20 UTC (permalink / raw)
To: Jes Sorensen
Cc: John Heenan, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <wrfj1sywrz2f.fsf@redhat.com>
On Mon, Oct 31, 2016 at 05:25:12PM -0400, Jes Sorensen wrote:
> As mentioned previously, if this is to be changed here, it has to be
> matched in the _stop section too. It also has to be investigated exactly
> why this matters for 8723bu. It is possible this matters for other
> devices, but we need to find out exactly what causes this not moving a
> major block of init code around like this.
>
I've tested this on the 8192e and 8192c. The same problems occurs with the
8192e but not the 8192c. Stopping and restarting wpa_supplicant had
no affect on the 8192c ability to connect to an AP.
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: Jes Sorensen @ 2016-10-31 22:41 UTC (permalink / raw)
To: Barry Day; +Cc: John Heenan, Kalle Valo, linux-wireless
In-Reply-To: <20161031222045.GA14900@testbox>
Barry Day <briselec@gmail.com> writes:
> On Mon, Oct 31, 2016 at 05:25:12PM -0400, Jes Sorensen wrote:
>> As mentioned previously, if this is to be changed here, it has to be
>> matched in the _stop section too. It also has to be investigated exactly
>> why this matters for 8723bu. It is possible this matters for other
>> devices, but we need to find out exactly what causes this not moving a
>> major block of init code around like this.
>
> I've tested this on the 8192e and 8192c. The same problems occurs with the
> 8192e but not the 8192c. Stopping and restarting wpa_supplicant had
> no affect on the 8192c ability to connect to an AP.
Which version of the driver? I did push in some changes for 8192eu
recently that fixed the issue with 8192eu driver reloading, and I would
be interested in knowing if this didn't fix the problem for you?
Second, could we please keep this on the linux-wireless list where it
belongs. Everybody else doesn't necessarily want to receive a copy via
netdev and linux-kernel
Jes
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: Barry Day @ 2016-10-31 23:47 UTC (permalink / raw)
To: Jes Sorensen; +Cc: John Heenan, Kalle Valo, linux-wireless
In-Reply-To: <wrfjoa20qgyt.fsf@redhat.com>
On Mon, Oct 31, 2016 at 06:41:30PM -0400, Jes Sorensen wrote:
> Barry Day <briselec@gmail.com> writes:
> > On Mon, Oct 31, 2016 at 05:25:12PM -0400, Jes Sorensen wrote:
> >> As mentioned previously, if this is to be changed here, it has to be
> >> matched in the _stop section too. It also has to be investigated exactly
> >> why this matters for 8723bu. It is possible this matters for other
> >> devices, but we need to find out exactly what causes this not moving a
> >> major block of init code around like this.
> >
> > I've tested this on the 8192e and 8192c. The same problems occurs with the
> > 8192e but not the 8192c. Stopping and restarting wpa_supplicant had
> > no affect on the 8192c ability to connect to an AP.
>
> Which version of the driver? I did push in some changes for 8192eu
> recently that fixed the issue with 8192eu driver reloading, and I would
> be interested in knowing if this didn't fix the problem for you?
>
> Second, could we please keep this on the linux-wireless list where it
> belongs. Everybody else doesn't necessarily want to receive a copy via
> netdev and linux-kernel
>
> Jes
The tests were done with the latest version of rtl8xxxu-devel. I haven't
noticed any occurence of the previous issue with the 8192eu.
^ permalink raw reply
* Re: [PATCH 01/12] mwifiex: fix power save issue when suspend
From: Brian Norris @ 2016-10-31 23:57 UTC (permalink / raw)
To: Xinming Hu
Cc: Linux Wireless, Kalle Valo, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Shengzhen Li
In-Reply-To: <1477900940-10549-1-git-send-email-huxinming820@marvell.com>
Hi,
On Mon, Oct 31, 2016 at 04:02:09PM +0800, Xinming Hu wrote:
> From: Shengzhen Li <szli@marvell.com>
>
> This patch fixes a corner case for "FROMLIST: mwifiex: fix corner case
Upstream patches don't normally get a 'FROMLIST' tag while you're
sending them to the mailing list :) That designation is used for Chrome
OS trees, so we know where to find the patch source. But the upstream
mailing list *is* the source.
> power save issue", main process will check the power save condition in
> PS_PRE_SLEEP status so the sleep handshake could continue.
>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Shengzhen Li <szli@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
>
> BUG=chrome-os-partner:58164
> TEST=stress Wifi w/ power_save enabled
>
> Change-Id: I5a36d9eaeb7fe5faaccc533e0d1ba1f3253666dc
In the same vein: while I'm happy to have the BUG=, TEST=, and Gerrit
Change-ID for our Chrome OS tree, I don't think upstream reviewers
typically care about those. Please remove those from upstream
submissions like this.
(Same applies to the rest of this series. Also, consider running
scripts/checkpatch.pl. It will at least remind you about removing the
Gerrit Change-ID.)
> ---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 3 ++-
> drivers/net/wireless/marvell/mwifiex/main.c | 5 +++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 5347728..9075be5 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -1123,8 +1123,9 @@ mwifiex_check_ps_cond(struct mwifiex_adapter *adapter)
> mwifiex_dnld_sleep_confirm_cmd(adapter);
> else
> mwifiex_dbg(adapter, CMD,
> - "cmd: Delay Sleep Confirm (%s%s%s)\n",
> + "cmd: Delay Sleep Confirm (%s%s%s%s)\n",
> (adapter->cmd_sent) ? "D" : "",
> + (adapter->data_sent) ? "T" : "",
> (adapter->curr_cmd) ? "C" : "",
> (IS_CARD_RX_RCVD(adapter)) ? "R" : "");
> }
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index 2478ccd..f559ead 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -308,6 +308,11 @@ process_start:
> /* We have tried to wakeup the card already */
> if (adapter->pm_wakeup_fw_try)
> break;
> + if (adapter->ps_state == PS_STATE_PRE_SLEEP) {
> + if (!adapter->cmd_sent && !adapter->curr_cmd)
The entire 'if' condition is unnecessary. That's all checked already
within mwifiex_check_ps_cond().
Brian
> + mwifiex_check_ps_cond(adapter);
> + }
> +
> if (adapter->ps_state != PS_STATE_AWAKE)
> break;
> if (adapter->tx_lock_flag) {
> --
> 1.8.1.4
>
^ permalink raw reply
* Re: [PATCH 02/12] mwifiex: check tx_hw_pending before downloading sleep confirm
From: Brian Norris @ 2016-11-01 0:14 UTC (permalink / raw)
To: Xinming Hu
Cc: Linux Wireless, Kalle Valo, Dmitry Torokhov, Amitkumar Karwar,
Cathy Luo, Shengzhen Li
In-Reply-To: <1477900940-10549-2-git-send-email-huxinming820@marvell.com>
On Mon, Oct 31, 2016 at 04:02:10PM +0800, Xinming Hu wrote:
> From: Shengzhen Li <szli@marvell.com>
>
> This patch will only allow downloading sleep confirm
> when no tx done interrupt is pending in the hardware.
>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Shengzhen Li <szli@marvell.com>
>
> Change-Id: I6d6955b4a2de0ad791ca28f0f635d636a2c7e406
^^ Remove this
> ---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 4 ++--
> drivers/net/wireless/marvell/mwifiex/init.c | 1 +
> drivers/net/wireless/marvell/mwifiex/main.h | 1 +
> drivers/net/wireless/marvell/mwifiex/pcie.c | 5 +++++
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 9075be5..25a7475 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -1118,14 +1118,14 @@ mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
> void
> mwifiex_check_ps_cond(struct mwifiex_adapter *adapter)
> {
> - if (!adapter->cmd_sent &&
> + if (!adapter->cmd_sent && !atomic_read(&adapter->tx_hw_pending) &&
How is this different from the following?
https://patchwork.kernel.org/patch/9389485/
And parituclarly, they conflict on this line, so you need to resolve
that somehow...
And if the linked patch serves the same purpose, then I'd rather take
it, since it's much simpler.
Brian
> !adapter->curr_cmd && !IS_CARD_RX_RCVD(adapter))
> mwifiex_dnld_sleep_confirm_cmd(adapter);
> else
> mwifiex_dbg(adapter, CMD,
> "cmd: Delay Sleep Confirm (%s%s%s%s)\n",
> (adapter->cmd_sent) ? "D" : "",
> - (adapter->data_sent) ? "T" : "",
> + atomic_read(&adapter->tx_hw_pending) ? "T" : "",
> (adapter->curr_cmd) ? "C" : "",
> (IS_CARD_RX_RCVD(adapter)) ? "R" : "");
> }
> diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
> index 82839d9..b36cb3f 100644
> --- a/drivers/net/wireless/marvell/mwifiex/init.c
> +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> @@ -270,6 +270,7 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
> adapter->adhoc_11n_enabled = false;
>
> mwifiex_wmm_init(adapter);
> + atomic_set(&adapter->tx_hw_pending, 0);
>
> sleep_cfm_buf = (struct mwifiex_opt_sleep_confirm *)
> adapter->sleep_cfm->data;
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index d61fe3a..7f67f23 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -857,6 +857,7 @@ struct mwifiex_adapter {
> atomic_t rx_pending;
> atomic_t tx_pending;
> atomic_t cmd_pending;
> + atomic_t tx_hw_pending;
> struct workqueue_struct *workqueue;
> struct work_struct main_work;
> struct workqueue_struct *rx_workqueue;
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 063c707..4aa5d91 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -516,6 +516,7 @@ static int mwifiex_pcie_enable_host_int(struct mwifiex_adapter *adapter)
> }
> }
>
> + atomic_set(&adapter->tx_hw_pending, 0);
> return 0;
> }
>
> @@ -689,6 +690,7 @@ static void mwifiex_cleanup_txq_ring(struct mwifiex_adapter *adapter)
> card->tx_buf_list[i] = NULL;
> }
>
> + atomic_set(&adapter->tx_hw_pending, 0);
> return;
> }
>
> @@ -1126,6 +1128,7 @@ static int mwifiex_pcie_send_data_complete(struct mwifiex_adapter *adapter)
> -1);
> else
> mwifiex_write_data_complete(adapter, skb, 0, 0);
> + atomic_dec(&adapter->tx_hw_pending);
> }
>
> card->tx_buf_list[wrdoneidx] = NULL;
> @@ -1218,6 +1221,7 @@ mwifiex_pcie_send_data(struct mwifiex_adapter *adapter, struct sk_buff *skb,
> wrindx = (card->txbd_wrptr & reg->tx_mask) >> reg->tx_start_ptr;
> buf_pa = MWIFIEX_SKB_DMA_ADDR(skb);
> card->tx_buf_list[wrindx] = skb;
> + atomic_inc(&adapter->tx_hw_pending);
>
> if (reg->pfu_enabled) {
> desc2 = card->txbd_ring[wrindx];
> @@ -1295,6 +1299,7 @@ mwifiex_pcie_send_data(struct mwifiex_adapter *adapter, struct sk_buff *skb,
> done_unmap:
> mwifiex_unmap_pci_memory(adapter, skb, PCI_DMA_TODEVICE);
> card->tx_buf_list[wrindx] = NULL;
> + atomic_dec(&adapter->tx_hw_pending);
> if (reg->pfu_enabled)
> memset(desc2, 0, sizeof(*desc2));
> else
> --
> 1.8.1.4
>
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: Joe Perches @ 2016-11-01 3:01 UTC (permalink / raw)
To: John Heenan, Jes Sorensen
Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <CAAye0QNtcb28q1HY-xAr=bQ0s_nEQuv1TM+Ax5i99Eds7xOb=Q@mail.gmail.com>
On Tue, 2016-11-01 at 08:15 +1000, John Heenan wrote:
> > On 1 November 2016 at 07:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> > > > John Heenan <john@zgus.com> writes:
> > > The rtl8723bu wireless IC shows evidence of a more agressive approach to
> > > power saving, powering down its RF side when there is no wireless
> > > interfacing but leaving USB interfacing intact. This makes the wireless
> > > IC more suitable for use in devices which need to keep their power use
> > > as low as practical, such as tablets and Surface Pro type devices.
> > >
> > > In effect this means that a full initialisation must be performed
> > > whenever a wireless interface is brought up. It also means that
> > > interpretations of power status from general wireless registers should
> > > not be relied on to influence an init sequence.
> > >
> > > The patch works by forcing a fuller initialisation and forcing it to
> > > occur more often in code paths (such as occurs during a low level
> > > authentication that initiates wireless interfacing).
> > >
> > > The initialisation sequence is now more consistent with code based
> > > directly on vendor code. For example while the vendor derived code
> > > interprets a register as indcating a particular powered state, it does
> > > not use this information to influence its init sequence.
> > >
> > > The rtl8723bu device has a unique USB VID and PID. This is taken
> > > advantage of for the patch to ensure only rtl8723bu devices are affected
> > > by this patch.
> > >
> > > With this patch wpa_supplicant reliably and consistently connects with
> > > an AP. Before a workaround such as executing rmmod and modprobe before
> > > each call to wpa_supplicant worked with some distributions.
> > >
> > > > > > Signed-off-by: John Heenan <john@zgus.com>
> > > ---
> > > .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 24 ++++++++++++++++++----
> > > 1 file changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > index 04141e5..f36e674 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > @@ -79,6 +79,8 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
> > > #define RTL8XXXU_TX_URB_LOW_WATER 25
> > > #define RTL8XXXU_TX_URB_HIGH_WATER 32
> > >
> > > +#define USB_PRODUCT_ID_RTL8723BU 0xb720
> > > +
> >
> > Absolutely not! You have no guarantee that this is the only id used for
> > 8723bu devices, and adding a #define for each is not going to happen.
>
> Thanks for you reply.
>
> I have no problem with that. However the patch does get the point
> across in a minimalist and efficient way of what the issues are.
>
> Currently there is no property available to determine the information required.
>
> >
> > > static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
> > > struct rtl8xxxu_rx_urb *rx_urb);
> > >
> > > @@ -3892,6 +3894,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> > > u8 val8;
> > > u16 val16;
> > > u32 val32;
> > > + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
> >
> > Indentaiton
>
> OK. Missed that one.
>
> >
> > > /* Check if MAC is already powered on */
> > > val8 = rtl8xxxu_read8(priv, REG_CR);
> > > @@ -3900,7 +3903,9 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> > > * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
> > > * initialized. First MAC returns 0xea, second MAC returns 0x00
> > > */
> > > - if (val8 == 0xea)
> > > + if (val8 == 0xea
> > > + || (udesc->idVendor == USB_VENDOR_ID_REALTEK
> > > + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU))
> > > macpower = false;
> > > else
> > > macpower = true;
> >
> > Please respect proper kernel coding style!
>
> I don't know what you mean. Your code has real tabs. My code has real
> tabs. The kernel style goes on about tabs being 8 spaces. So do you
> want: real tabs or real spaces?
>
> You said no lines over 80 columns long. This is what i have done.
Typical kernel style would be:
if (val == 0xea ||
(udesc->idVendor == USB_VENDOR_ID_REALTEK &&
udesc->idProduct == USB_PRODUCT_ID_RTL8723BU))
macpower = false;
else
macpower = true;
ie: logical continuations at EOL and indentation aligned to parentheses
using as many leading tabs as possible, then spaces where necessary
or maybe:
macpower = !(val == 0xea ||
(idesc->idVendor == USB_VENDOR_ID_REALTEK &&
udesc->idProduct == USB_PRODUCT_ID_RTL8723BU));
but the first one seems easier to read.
> > > @@ -6080,9 +6093,12 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> > > goto exit;
> > > }
> > >
> > > - ret = rtl8xxxu_init_device(hw);
> > > - if (ret)
> > > + if(!(id->idVendor == USB_VENDOR_ID_REALTEK
> > > + && id->idProduct == USB_PRODUCT_ID_RTL8723BU)) {
> > > + ret = rtl8xxxu_init_device(hw);
> > > + if (ret)
> > > goto exit;
> > > + }
> >
> > Again, this coding style abuse will never go into this driver,
>
> As above, what abuse? I am not being facetious. Just puzzled.
Same logical continuation and indentation alignment.
> Again have a nice day!
That's pleasant of you but Jen's rarely seems pleasant in return via
email. I trust he's more personable over a beer though. Perhaps one
day we'll all have one together.
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2016-10-30
From: Kalle Valo @ 2016-11-01 5:25 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20161031.153809.270896325416087100.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Kalle Valo <kvalo@codeaurora.org>
> Date: Sun, 30 Oct 2016 11:20:46 +0200
>
>> few fixes for 4.9. I tagged this on the plane over a slow mosh
>> connection while travelling to Plumbers so I might have done something
>> wrong, please check more carefully than usually. For example I had to
>> redo the signed tag because of some whitespace damage.
>>
>> Please let me know if there are any problems.
>
> Your subject line says "-next" but clearly these are bug fixes for 'net'
> so that's where I pulled this into.
Correct, that -next was a mistake. Sorry about that.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-11-01 7:24 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <CAAye0QNtcb28q1HY-xAr=bQ0s_nEQuv1TM+Ax5i99Eds7xOb=Q@mail.gmail.com>
I have a prepared another patch that is not USB VID/PID dependent for
rtl8723bu devices. It is more elegant. I will send it after this
email.
If I have more patches is it preferable I just put them on github only
and notify a link address until there might be some resolution?
What I meant below about not finding a matching function is that I
cannot find matching actions to take on any function calls in
rtl8xxxu_stop as for rtl8xxxu_start.
The function that is called later and potentially more than once for
rtl8723bu devices is rtl8xxxu_init_device. There is no corresponding
rtl8xxxu_deinit_device function.
enable_rf is called in rtl8xxxu_start, not in the delayed call to
rtl8xxxu_init_device. The corresponding disable_rf is called in
rtl8xxxu_stop. So no matching issue here. However please see below for
another potential issue.
power_on is called in rtl8xxxu_init_device. power_off is called in
rtl8xxxu_disconnect. Does not appear to be an issue if calling
power_on has no real effect if already on.
The following should be looked at though. For all devices enable_rf is
only called once. For the proposed patch the first call to
rtl8xxxu_init_device is called after the single call to enable_rf for
rtl8723bu devices. Without the patch enable_rf is called after
rtl8xxxu_init_device for all devices
Perhaps enable_rf just configures RF modes to start up when RF is
powered on or if called after power_on then enters requested RF modes.
So I cannot see appropriate additional matching action to take.
Below is some background for anyone interested
rtl8xxxu_start and rtl8xxxu_stop are assigned to rtl8xxxu_ops.
rtl8xxxu_probe assigns rtl8xxxu_ops to its driver layer with
ieee80211_register_hw.
rtl8xxxu_disconnect unassigns rtl8xxxu_ops from its driver layer with
ieee80211_unregister_hw.
rtl8xxxu_probe and rtl8xxxu_disconnect are USB driver functions
John Heenan
On 1 November 2016 at 08:15, John Heenan <john@zgus.com> wrote:
> On 1 November 2016 at 07:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>> John Heenan <john@zgus.com> writes:
>
>>
>>> @@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>>> struct rtl8xxxu_tx_urb *tx_urb;
>>> unsigned long flags;
>>> int ret, i;
>>> + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
>>>
>>> ret = 0;
>>>
>>> + if(udesc->idVendor == USB_VENDOR_ID_REALTEK
>>> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
>>> + ret = rtl8xxxu_init_device(hw);
>>> + if (ret)
>>> + goto error_out;
>>> + }
>>> +
>>
>> As mentioned previously, if this is to be changed here, it has to be
>> matched in the _stop section too.
>
> I looked at this and could not find a matching function. I will have a
> look again.
>
^ permalink raw reply
* [PATCH v2] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-11-01 7:24 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless, netdev; +Cc: linux-kernel
The rtl8723bu wireless IC shows evidence of a more agressive approach to
power saving, powering down its RF side when there is no wireless
interfacing but leaving USB interfacing intact. This makes the wireless
IC more suitable for use in devices which need to keep their power use
as low as practical, such as tablets and Surface Pro type devices.
In effect this means that a full initialisation must be performed
whenever a wireless interface is brought up. It also means that
interpretations of power status from general wireless registers should
not be relied on to influence an init sequence.
The patch works by forcing a fuller initialisation and forcing it to
occur more often in code paths (such as occurs during a low level
authentication that initiates wireless interfacing).
The initialisation sequence is now more consistent with code based
directly on vendor code. For example while the vendor derived code
interprets a register as indcating a particular powered state, it does
not use this information to influence its init sequence.
Only devices that use the rtl8723bu driver are affected by this patch.
With this patch wpa_supplicant reliably and consistently connects with
an AP. Before a workaround such as executing rmmod and modprobe before
each call to wpa_supplicant worked with some distributions.
Signed-off-by: John Heenan <john@zgus.com>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 04141e5..ab2f2ef 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3900,7 +3900,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
* Fix 92DU-VC S3 hang with the reason is that secondary mac is not
* initialized. First MAC returns 0xea, second MAC returns 0x00
*/
- if (val8 == 0xea)
+ if (val8 == 0xea || priv->fops == &rtl8723bu_fops)
macpower = false;
else
macpower = true;
@@ -5779,6 +5779,12 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
ret = 0;
+ if(priv->fops == &rtl8723bu_fops) {
+ ret = rtl8xxxu_init_device(hw);
+ if (ret)
+ goto error_out;
+ }
+
init_usb_anchor(&priv->rx_anchor);
init_usb_anchor(&priv->tx_anchor);
init_usb_anchor(&priv->int_anchor);
@@ -6080,9 +6086,11 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
goto exit;
}
- ret = rtl8xxxu_init_device(hw);
- if (ret)
- goto exit;
+ if(priv->fops != &rtl8723bu_fops) {
+ ret = rtl8xxxu_init_device(hw);
+ if (ret)
+ goto exit;
+ }
hw->wiphy->max_scan_ssids = 1;
hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
--
2.10.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox